chore: sync with upstream 2026-07-27 - #85
Merged
Merged
Conversation
## Summary - add CP-owned admission policy with existing OR semantics, deny-by-default behavior, complete verified-email evaluation, and same-flow GitHub organization membership checks - resolve browser identities only by canonical issuer plus exact immutable provider subject, with provenance-bearing verified-email reservations and explicit account-link-required collisions - orchestrate consumed provider callbacks through provider-specific callback handlers, admission, identity resolution, credential capture, and exact client-bound authorization-code issuance - separate browser sign-in policy from persistence: the resolver owns evidence, collision, and retry policy while the DB store owns row decoding and atomic multi-table writes - add a GitHub App permission preflight that verifies both registration and installation permissions before cutover ## Security properties - revalidates the persisted client and exact redirect URI after single-use state consumption and before provider exchange - isolates provider-specific callback mechanics (including Google's OIDC nonce binding) behind an exhaustive provider-handler registry; adding Okta requires an explicit handler instead of falling through to another provider - keeps Google credentials out of storage and uses the maintained provider adapters from ColeMurray#1118 for OAuth and OIDC protocol validation - preserves provider subjects exactly; identity remains the case-sensitive `(issuer, subject)` tuple - never reparents an established immutable subject based on email - creates each new user, issuer-qualified identity, verified-email claims, and GitHub provider credential in one atomic D1 batch - refreshes existing identity metadata, claims, and versioned provider credentials in one retryable D1 batch - bounds verified-email evidence and uses JSON-table statements so a valid GitHub result cannot exceed D1's 100-bind limit or require one statement per email - retries uniqueness races by re-reading issuer-subject and claim ownership; it never falls back to an email owner - preserves legacy canonical-email claim provenance while allowing current provider claims to advance verification timestamps - maps provider, admission, and collision failures to bounded callback errors without retaining OAuth state, raw causes, tokens, provider bodies, emails, subjects, or authorization codes - makes GitHub `email_addresses: read` an intrinsic preflight requirement because sign-in always reads `/user/emails` ## Scope This PR adds inert control-plane domain services and tests only. It does not expose routes, change cookies, alter deployment configuration, or activate the new authentication path. HTTP parsing, rate limiting, telemetry, composition, and cutover wiring remain follow-up work. Account linking remains intentionally deferred. A new subject that collides with another trusted email reservation fails with `account_link_required` and creates nothing. ## TDD and validation Red-green-refactor coverage includes admission parity, provider-mixup rejection, exact redirect rebinding, provider-handler dispatch, bounded and sanitized provider failures, exact subject preservation, immutable identity races, bounded D1-safe email fan-out, schema-pinned stale-credential retry and atomic rollback, email collisions, legacy claim provenance, atomic credential capture and rollback, Google credential rejection, and independent App/installation permission and suspension checks. The real D1 integration path proves provider callback -> canonical user and credential -> authorization code -> browser session redemption and authentication. - `npm test -w @open-inspect/control-plane` (141 files, 2,174 tests) - `npm run test:integration -w @open-inspect/control-plane` (61 files, 714 tests) - `npm run typecheck` - `npm run lint -w @open-inspect/control-plane` - `npm run format:check` - `npm run build -w @open-inspect/shared` - `npm run build -w @open-inspect/control-plane` - `git diff --check` <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit * **New Features** * Added configurable sign-in admission policies (email, domain, GitHub user, and optional GitHub organization checks) with “unsafe allow all” behavior. * Added GitHub App permission preflight validation for app JWT + installation permission readiness. * Implemented OAuth callback handling for both GitHub and Google, including correct success/denial redirects. * Strengthened browser sign-in identity resolution and verified-email handling with account-linking safeguards. * **Bug Fixes** * Improved denial/unavailable vs server error classification, redirect safety, and failure rollback behavior. * **Tests** * Expanded unit and integration coverage across admission, OAuth callbacks, identity persistence, and credential concurrency/version conflicts. <!-- end of auto-generated comment: release notes by coderabbit.ai -->
## Summary - increase the control-plane integration job timeout from five to ten minutes - leave every test command and all other CI limits unchanged ## Why The complete integration suite now runs close to five minutes after dependency installation and workerd setup. A production validation run reached the existing job limit and GitHub cancelled the still-running test process without a failing assertion. Ten minutes preserves a finite bound while allowing the suite to report its actual result. ## Impact This changes CI scheduling only. It does not change application code, deployment behavior, or test semantics. ## Validation - Prettier check passed for the workflow - git diff --check passed - the full control-plane integration suite passed locally before this timeout adjustment <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit * **Chores** * Increased the integration test workflow timeout to reduce failures caused by slow test runs. <!-- end of auto-generated comment: release notes by coderabbit.ai -->
## Summary - pin Better Auth 1.6.25 in the control plane - add the minimal control-plane-owned browser-auth configuration boundary - make Better Auth generate canonical Open Inspect user IDs - add the additive Better Auth user, account, session, and verification schema - enforce uniqueness for immutable provider identities - make the pinned runtime validate the complete static schema contract in workerd with real D1 ## Why This is the inert foundation for replacing the custom browser OAuth runtime with the Better Auth implementation already validated in production. No routes call this configuration yet, so this PR does not change current browser authentication behavior. The schema is intentionally browser-specific and additive. It does not predict CLI, PAT, MFA, magic-link, or future Okta storage. Implicit account linking is disabled; explicit linking remains follow-up work. Before routes activate, Better Auth user IDs are projected unchanged into canonical users.id and auth_accounts becomes the browser-provider credential authority; the legacy credential table is not dual-written. ## Rollout The CI timeout prerequisite in ColeMurray#1124 has merged. This branch is rebased directly on current main and contains only the Better Auth runtime and schema foundation. ## Validation - control-plane lint passed - control-plane typecheck passed - control-plane build passed - 141 control-plane unit files, 2,175 tests passed - 62 control-plane integration files, 716 tests passed - focused Better Auth workerd/D1 integration: 3 tests passed - schema contract checks cover pinned-runtime drift, every column shape, foreign keys, and the custom provider-identity index - git diff --check passed ## Dependency audit The production audit reports the existing public-main Next.js/PostCSS/sharp advisories. Better Auth adds another dependency path to the same installed Next version; it does not introduce a new advisory or a second vulnerable package version. <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit * **New Features** * Enabled browser authentication for the control plane with secure, fixed cookie behavior. * Added the core authentication database schema for users, sessions, linked accounts, and verification records. * Added an API behavior for anonymous session checks (returns `200` with `null` session). * **Bug Fixes** * Enforced provider/account uniqueness, email uniqueness, and cascading cleanup for user-related auth records. * **Tests** * Added integration coverage to validate the browser-auth runtime schema and request/session boundary behavior. <!-- end of auto-generated comment: release notes by coderabbit.ai -->
) ## Summary Completes the browser-authentication cutover that began with ColeMurray#1125. The control plane now owns Better Auth, provider verification, admission, canonical user projection, sessions, and provider credentials. The Next.js app is a framework-free BFF that forwards an exact auth-route allowlist and authenticates application requests with both the web service's `sig1` channel and the browser's opaque session cookie. This is intentionally one complete cutover PR. The implementation has already been deployed and exercised in the production repository, including the production fixes discovered during that rehearsal. Merging it as one unit avoids leaving public `main` with overlapping Better Auth, NextAuth, bearer-token, and hand-rolled OAuth architectures. ## Resulting architecture 1. The browser calls the web application's `/api/auth/*` routes. 2. The web BFF forwards only the shared exact Better Auth route allowlist. 3. Every forwarded request is body-bound and signed as `service:web` with `sig1`. 4. The control plane verifies GitHub or Google identity evidence and applies admission policy. 5. Better Auth owns browser users, provider accounts, OAuth credentials, and opaque sessions in D1. 6. Better Auth user IDs are projected unchanged into canonical `users.id`. 7. Browser resource requests require both: - a valid `service:web` channel; and - a valid Better Auth session cookie. 8. Authorization receives `{ kind: "user", userId }`; provider provenance remains in the authentication context. 9. GitHub access and refresh remain Better Auth-owned. Session creation/prompt flows request a current access token through Better Auth and never copy the long-lived refresh token. ## Review guide The commits remain logically ordered so this large PR can be reviewed as vertical slices: 1. **Provider identity and admission** - defensive GitHub response validation and bounded pagination/retry behavior; - Google ID-token verification using Better Auth's provider implementation; - verified-email, GitHub-user, email-domain, and GitHub-organization admission. 2. **Signed auth surface and BFF proxy** - one shared exact route allowlist; - control-plane `sig1` enforcement; - transparent callback, cookie, redirect, and decoded-body header handling. 3. **Canonical identity and compound authentication** - canonical ID generation and user projection; - provider provenance separated from the authorized principal; - browser session plus web-channel validation. 4. **Web cutover and provider credentials** - Better Auth sign-in, sign-out, and session consumption; - authenticated BFF resource requests; - Better Auth-owned GitHub credential refresh and attribution. 5. **Deployment and legacy removal** - control-plane provider/auth configuration; - removal of NextAuth, web bearer tokens, and the superseded custom OAuth server; - provider secrets and admission policy removed from the web runtime. 6. **Production-rehearsal fixes** - stale response encoding/length headers; - bound Workers `fetch`; - legacy-user collision cleanup and account backfill; - multi-provider session handling; - workerd-compatible package resolution; - explicit rate limiting and trusted client-IP propagation. The final reconciliation commits remove custom OAuth files that were merged into public `main` after the production validation branch was created, remove the unused direct OAuth client dependency, and retain the schema-alignment and duration-unit improvements from ColeMurray#1125. The latest organization pass groups user and service authentication by responsibility, moves GitHub credential authority into source control, and narrows Better Auth dependencies at both boundaries. ## Migration and deployment behavior - `0049_backfill_better_auth_accounts.sql` idempotently seeds Better Auth users and immutable GitHub/Google accounts from the existing canonical model. - The backfill removes only partial Better Auth identity graphs left by the accepted non-atomic D1 adapter behavior when an existing canonical email caused projection to fail. - Existing provider access and refresh tokens are not copied. The next successful sign-in captures fresh Better Auth-owned credentials. - Existing browser sessions are intentionally invalidated; users sign in again after cutover. - The existing Terraform `nextauth_secret` input and Actions `NEXTAUTH_SECRET` name are retained as operator-facing compatibility names, but now supply control-plane `BROWSER_AUTH_SECRET`. - GitHub and Google callback URLs remain on the browser-visible web origin: - `<web-origin>/api/auth/callback/github` - `<web-origin>/api/auth/callback/google` - Legacy custom-auth D1 tables remain additive residue for now, but no runtime route or service consumes them. For existing Cloudflare web deployments, the uploader no longer sends OAuth or NextAuth secrets, but Wrangler preserves already-uploaded secrets. Operators should delete stale `GITHUB_CLIENT_SECRET`, `GOOGLE_CLIENT_SECRET`, and `NEXTAUTH_SECRET` values from the web Worker after validating the cutover. ## Security properties - Provider identities are immutable `(provider, subject)` accounts. - GitHub and Google successful responses are runtime-validated before admission. - Implicit account linking is disabled. - OAuth provider secrets and admission allowlists exist only in the control plane. - Browser resource routes have no service-credential-to-user fallback. - The BFF forwards only the opaque Better Auth session cookie and rejects malformed or duplicate session-cookie data. - Auth proxy and resource request bodies are covered by `sig1`. - Production cookies are secure, HTTP-only, host-only, and `SameSite=Lax`; insecure cookies are limited to exact loopback HTTP development origins. - Retired bearer-token and provider-identity endpoints are no longer routed. ## Accepted scope decisions - Better Auth's exact-pinned D1 adapter does not provide an interactive transaction. Initial sign-in can therefore leave recoverable partial state if a later write fails. This rollout accepts that risk; the migration repairs the known legacy-email collision. - Explicit account linking is deferred. The canonical model supports a future linking operation, but implicit email linking remains disabled. - Better Auth's in-memory limiter is best-effort per Worker isolate, not a global abuse-control system. - Better Auth stores its random session identifier in D1 and authenticates the browser cookie with `BROWSER_AUTH_SECRET`; this is not the literal hash-at-rest representation from the superseded custom design. ## Validation - Control-plane focused auth unit tests: **23 files, 207 tests passed** - Control-plane focused workerd/D1 integration tests: **4 files, 28 tests passed** - Control-plane unit tests: **136 files, 2,111 tests passed** - Control-plane integration tests: **57 files, 653 tests passed** - Web tests: **101 files, 790 tests passed** - Shared tests: **36 files, 489 tests passed** - GitHub bot tests: **7 files, 128 tests passed** - Slack bot tests: **28 files, 338 tests passed** - Linear bot tests: **13 files, 185 tests passed** - Full TypeScript typecheck: passed - Full ESLint: passed - Full workspace build, including the production web build: passed - Terraform recursive formatting check: passed - Terraform initialization and validation: passed - Thermo-nuclear maintainability review: passed with scoped cleanup applied <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit * **New Features** * Rolled out control-plane–driven browser authentication (GitHub, with optional Google) and a secure browser-auth proxy for web requests, enabling authenticated enrichment for session-related flows. * **Bug Fixes** * Prevented forwarding stale `Content-Length`/`Content-Encoding` when returning decoded streamed content (media/attachments/session diffs). * Improved access-denied messaging to consistently handle both legacy and new denial codes. * **Documentation** * Updated setup/deployment guides and environment examples to use the new browser-auth secret terminology and configuration expectations. <!-- end of auto-generated comment: release notes by coderabbit.ai -->
This is an automated nightly unsafe-cast remediation sweep. It replaces selected high-risk Linear bot boundary assertions with parse-don't-assert validation, following the TypeScript Coding Standards for unsafe casts and the Zod boundary-validation pattern established in PR ColeMurray#807. | Finding | Risk | Cast removed | Fix | | --- | --- | --- | --- | | `packages/linear-bot/src/webhook-handler.ts:177` | HIGH | Control-plane create-session response cast to `{ sessionId: string }`, bypassing the existing shared schema | Reused `createSessionResponseSchema.safeParse`; malformed success responses now follow the existing create-session failure path | | `packages/linear-bot/src/webhook-handler.ts:380` | HIGH | Control-plane session events response cast to `Array<{ type; data }>` feeding follow-up prompt context | Added a local Zod response schema for the consumed event fields and skip prior-context enrichment when parsing fails | | `packages/linear-bot/src/utils/integration-config.ts:58` | HIGH | Control-plane resolved Linear config response cast to `ResolvedLinearConfig` | Added a package-local Zod schema and made `z.infer` the config type source of truth; malformed responses fall back to defaults | Verification: | Command | Result | | --- | --- | | `npm run build -w @open-inspect/shared` | Passed | | `npm run build -w @open-inspect/linear-bot` | Passed | | `npm run typecheck` | Passed | | `npm test -w @open-inspect/linear-bot` | Passed, 13 files / 189 tests | | `npm run lint -w @open-inspect/linear-bot` | Passed | | `npm run lint -- --ignore-pattern '.opencode/**'` | Passed for tracked repo code; literal `npm run lint` in this workspace is blocked by a pre-existing untracked `.opencode/` directory that is not part of this PR | | `npm run format` | Passed | --- *Created with [Open-Inspect](https://open-inspect-prod.vercel.app/session/feca6455e6cb748e1ada460d645a4c87)* --------- Co-authored-by: OpenInspect <open-inspect@noreply.github.com> Co-authored-by: Cole Murray <colemurray.cs@gmail.com>
This is an automated nightly unsafe-cast remediation. It replaces unsafe assertions at an external Anthropic API boundary with package-local Zod validation, following the TypeScript Coding Standards unsafe-cast / parse-don't-assert guidance and the Zod boundary-validation pattern established in PR ColeMurray#807. | Finding | Risk | Cast removed | Fix | | --- | --- | --- | --- | | `packages/linear-bot/src/classifier/index.ts:141` | HIGH | `(await response.json()) as AnthropicResponse` on an external API response | Added `anthropicMessagesResponseSchema` and `safeParse` before reading `content` | | `packages/linear-bot/src/classifier/index.ts:148` | HIGH | `toolBlock.input as Record<string, unknown>` plus `input.confidence as ConfidenceLevel` | Added `classifyToolInputSchema` with `repoId` modeled as `string | null`, confidence enum validation, and `safeParse` before returning the `z.infer` type | Verification: | Command | Result | | --- | --- | | `npm run build -w @open-inspect/shared` | Passed | | `npm run build -w @open-inspect/linear-bot` | Passed | | `npm test -w @open-inspect/linear-bot` | Passed, 200 tests | | `npm run typecheck` | Passed | | `npm run lint -w @open-inspect/linear-bot` | Passed | | `npm run format` | Passed | | `npm run lint` | Passed in a clean worktree for this commit; the active agent workspace contains untracked local `.opencode` tool files that are not part of this PR and cause root lint false positives there | --- *Created with [Open-Inspect](https://open-inspect-prod.vercel.app/session/8c9388c89bcb8c9de06d4b25235c0809)* Co-authored-by: waclaude <colemurray.cs+ghwaclaude@gmail.com>
Terraform Validation Results
Pushed by: @NicolasWalter, Action: |
📝 WalkthroughWalkthroughThis PR replaces web-owned NextAuth and opaque session tokens with Better Auth browser sessions, signed web-service requests, control-plane admission, provider enrichment, new auth persistence, and updated web/deployment configuration. It also adds response validation and removes retired authentication routes. ChangesBrowser authentication cutover
Estimated code review effort: 5 (Critical) | ~120 minutes Possibly related PRs
Suggested reviewers: 🚥 Pre-merge checks | ✅ 3 | ❌ 2❌ Failed checks (1 warning, 1 inconclusive)
✅ Passed checks (3 passed)
✨ Finishing Touches📝 Generate docstrings
Warning There were issues while running some tools. Please review the errors and either fix the tool's configuration or disable the tool if it's a critical failure. 🔧 Trivy (0.72.0)Trivy execution failed: 2026-07-27T06:22:30Z FATAL Fatal error run error: fs scan error: scan error: scan failed: failed analysis: post analysis error: post analysis error: kubernetes scan error: fs filter error: fs filter error: walk error range error: stat doctor.config.json: no such file or directory: range error: stat doctor.config.json: no such file or directory Comment |
Terraform Plan ResultsStatus: ✅ Success Show Planterraform_data.cloudflare_custom_domain_gate: Refreshing state... [id=fa456fac-6c14-16e4-a484-3338d1a3718d]
terraform_data.access_control_gate: Refreshing state... [id=841ab6bc-98a7-018c-1031-ebad4f8b62bc]
data.external.modal_source_hash[0]: Reading...
null_resource.github_bot_build[0]: Refreshing state... [id=8669455087823957968]
null_resource.linear_bot_build[0]: Refreshing state... [id=5896821814655107141]
null_resource.slack_bot_build[0]: Refreshing state... [id=3828387022104391843]
random_password.service_auth_secret_linear_bot: Refreshing state... [id=none]
random_password.service_auth_secret_web: Refreshing state... [id=none]
random_password.service_auth_secret_github_bot: Refreshing state... [id=none]
random_password.service_auth_secret_slack_bot: Refreshing state... [id=none]
random_password.service_auth_secret_modal: Refreshing state... [id=none]
random_password.image_callback_token_pepper: Refreshing state... [id=none]
null_resource.web_app_cloudflare_build[0]: Refreshing state... [id=5412068031001148538]
null_resource.control_plane_build: Refreshing state... [id=8869648852438369746]
local_file.web_app_wrangler_production[0]: Refreshing state... [id=f3ffb89a3b5665e9d54b7790151b3b4b01b357f7]
module.linear_kv[0].cloudflare_workers_kv_namespace.this: Refreshing state... [id=d003f1ad81384910a1f48a0a33f18c09]
cloudflare_d1_database.main: Refreshing state... [id=dba95b03-ace9-47a8-81e9-6e39d8d694c5]
cloudflare_r2_bucket.media: Refreshing state... [id=open-inspect-media-primo]
module.github_kv[0].cloudflare_workers_kv_namespace.this: Refreshing state... [id=87dbfaa1d9ce4a37a42e04c57c434a72]
cloudflare_queue.slack_completion_delivery_dlq[0]: Refreshing state... [id=06ce03d2663f4aea937b0c0c1c379c17]
module.slack_kv[0].cloudflare_workers_kv_namespace.this: Refreshing state... [id=729b357dbb5e4c9d99ec9212cc45766e]
module.session_index_kv.cloudflare_workers_kv_namespace.this: Refreshing state... [id=7f18644fbed34121bbe3a196f373ea93]
cloudflare_queue.slack_completion_delivery[0]: Refreshing state... [id=56ef0f3e13bd46a3a39f30c79ec547fa]
module.modal_app[0].null_resource.modal_secrets[0]: Refreshing state... [id=2786381799123975256]
data.external.modal_source_hash[0]: Read complete after 0s [id=-]
module.modal_app[0].null_resource.modal_deploy: Refreshing state... [id=7095165949869290970]
module.slack_bot_worker[0].cloudflare_worker.this: Refreshing state... [id=5200e96d69804ea296e1f3a6b39e4243]
module.linear_bot_worker[0].cloudflare_worker.this: Refreshing state... [id=33782d80e8ff4af9b30b92870084b674]
null_resource.d1_migrations: Refreshing state... [id=7966128981626684664]
module.linear_bot_worker[0].cloudflare_worker_version.this: Refreshing state... [id=16b53281-fe2f-4885-a89e-c2239de3b811]
module.slack_bot_worker[0].cloudflare_worker_version.this: Refreshing state... [id=6551b217-d1f4-4f1d-8072-253ad7a0d163]
module.linear_bot_worker[0].cloudflare_workers_deployment.this: Refreshing state... [id=5dc80780-d35d-46b2-8483-290d83a77ea2]
module.slack_bot_worker[0].cloudflare_workers_deployment.this: Refreshing state... [id=21642fd7-f856-48de-a26d-d501d8817102]
module.control_plane_worker.cloudflare_worker.this: Refreshing state... [id=c208a60c393e45e38eb502346bb7ce1e]
cloudflare_queue_consumer.slack_completion_delivery[0]: Refreshing state...
module.control_plane_worker.cloudflare_worker_version.this: Refreshing state... [id=5a41bfd5-5d1b-4f9e-a282-9235a9ad3bd9]
module.control_plane_worker.cloudflare_workers_deployment.this: Refreshing state... [id=6c613670-4aa5-41bd-bb95-206aee4ccd49]
module.control_plane_worker.cloudflare_workers_cron_trigger.this[0]: Refreshing state... [id=open-inspect-control-plane-primo]
null_resource.web_app_cloudflare_deploy[0]: Refreshing state... [id=7223362243013327446]
module.github_bot_worker[0].cloudflare_worker.this: Refreshing state... [id=4b5e2696491a41eaaa124f4e2a9855f2]
null_resource.web_app_cloudflare_secrets[0]: Refreshing state... [id=2052594936711735049]
module.github_bot_worker[0].cloudflare_worker_version.this: Refreshing state... [id=7aaed149-c286-4edb-9ace-310ce4d3ccd1]
module.github_bot_worker[0].cloudflare_workers_deployment.this: Refreshing state... [id=2129eb15-5cf8-47a9-9f96-ea9e5ddae4e6]
Terraform used the selected providers to generate the following execution
plan. Resource actions are indicated with the following symbols:
+ create
~ update in-place
-/+ destroy and then create replacement
Terraform will perform the following actions:
# local_file.web_app_wrangler_production[0] will be created
+ resource "local_file" "web_app_wrangler_production" {
+ content = (sensitive value)
+ content_base64sha256 = (known after apply)
+ content_base64sha512 = (known after apply)
+ content_md5 = (known after apply)
+ content_sha1 = (known after apply)
+ content_sha256 = (known after apply)
+ content_sha512 = (known after apply)
+ directory_permission = "0777"
+ file_permission = "0777"
+ filename = "../../..//packages/web/wrangler.production.toml"
+ id = (known after apply)
}
# null_resource.control_plane_build must be replaced
-/+ resource "null_resource" "control_plane_build" {
~ id = "8869648852438369746" -> (known after apply)
~ triggers = { # forces replacement
~ "always_run" = "2026-07-26T06:18:24Z" -> (known after apply)
}
}
# null_resource.d1_migrations must be replaced
-/+ resource "null_resource" "d1_migrations" {
~ id = "7966128981626684664" -> (known after apply)
~ triggers = { # forces replacement
~ "migrations_sha" = "803b74bf318acf1260ff63e62011811a74b0a1bdab14c52ae8bfe06f55bfff9f" -> "c208bd784688703e385cfeb4b4d09724f1ad2119d0ef2d70dc15c37a2a51bdf6"
# (1 unchanged element hidden)
}
}
# null_resource.github_bot_build[0] must be replaced
-/+ resource "null_resource" "github_bot_build" {
~ id = "8669455087823957968" -> (known after apply)
~ triggers = { # forces replacement
~ "always_run" = "2026-07-26T06:18:24Z" -> (known after apply)
}
}
# null_resource.linear_bot_build[0] must be replaced
-/+ resource "null_resource" "linear_bot_build" {
~ id = "5896821814655107141" -> (known after apply)
~ triggers = { # forces replacement
~ "always_run" = "2026-07-26T06:18:24Z" -> (known after apply)
}
}
# null_resource.slack_bot_build[0] must be replaced
-/+ resource "null_resource" "slack_bot_build" {
~ id = "3828387022104391843" -> (known after apply)
~ triggers = { # forces replacement
~ "always_run" = "2026-07-26T06:18:24Z" -> (known after apply)
}
}
# null_resource.web_app_cloudflare_build[0] must be replaced
-/+ resource "null_resource" "web_app_cloudflare_build" {
~ id = "5412068031001148538" -> (known after apply)
~ triggers = { # forces replacement
~ "always_run" = "2026-07-26T06:18:24Z" -> (known after apply)
}
}
# null_resource.web_app_cloudflare_deploy[0] must be replaced
-/+ resource "null_resource" "web_app_cloudflare_deploy" {
~ id = "7223362243013327446" -> (known after apply)
~ triggers = { # forces replacement
~ "always_run" = "2026-07-26T06:19:07Z" -> (known after apply)
}
}
# null_resource.web_app_cloudflare_secrets[0] must be replaced
-/+ resource "null_resource" "web_app_cloudflare_secrets" {
~ id = "2052594936711735049" -> (known after apply)
~ triggers = { # forces replacement
~ "secrets_hash" = (sensitive value)
}
}
# module.control_plane_worker.cloudflare_worker.this will be updated in-place
~ resource "cloudflare_worker" "this" {
id = "c208a60c393e45e38eb502346bb7ce1e"
name = "open-inspect-control-plane-primo"
~ observability = {
~ logs = {
+ destinations = (known after apply)
# (4 unchanged attributes hidden)
}
~ traces = {
+ destinations = (known after apply)
# (3 unchanged attributes hidden)
}
# (2 unchanged attributes hidden)
}
~ references = {
~ dispatch_namespace_outbounds = [] -> (known after apply)
~ domains = [] -> (known after apply)
~ durable_objects = [
- {
- namespace_id = "4c77239db3614a6aac69a90e1fbd8955" -> null
- namespace_name = "open-inspect-control-plane-primo_SessionDO" -> null
- worker_id = "c208a60c393e45e38eb502346bb7ce1e" -> null
- worker_name = "open-inspect-control-plane-primo" -> null
},
- {
- namespace_id = "bf3328c8ebcb4039855ed7fcca6eb7e9" -> null
- namespace_name = "open-inspect-control-plane-primo_SchedulerDO" -> null
- worker_id = "c208a60c393e45e38eb502346bb7ce1e" -> null
- worker_name = "open-inspect-control-plane-primo" -> null
},
] -> (known after apply)
~ queues = [] -> (known after apply)
~ workers = [
- {
- id = "33782d80e8ff4af9b30b92870084b674" -> null
- name = "open-inspect-linear-bot-primo" -> null
},
- {
- id = "5200e96d69804ea296e1f3a6b39e4243" -> null
- name = "open-inspect-slack-bot-primo" -> null
},
- {
- id = "ac07332f8b0f4cdfa4ca04f966a9fa61" -> null
- name = "open-inspect-web-primo" -> null
},
- {
- id = "4b5e2696491a41eaaa124f4e2a9855f2" -> null
- name = "open-inspect-github-bot-primo" -> null
},
] -> (known after apply)
} -> (known after apply)
tags = []
~ updated_on = "2026-07-26T06:18:33Z" -> (known after apply)
# (6 unchanged attributes hidden)
}
# module.control_plane_worker.cloudflare_worker_version.this must be replaced
-/+ resource "cloudflare_worker_version" "this" {
~ annotations = {
+ workers_message = (known after apply)
+ workers_tag = (known after apply)
~ workers_triggered_by = "create_version_api" -> (known after apply)
} -> (known after apply)
~ bindings = (sensitive value) # forces replacement
~ created_on = "2026-07-26T06:18:35Z" -> (known after apply)
~ id = "5a41bfd5-5d1b-4f9e-a282-9235a9ad3bd9" -> (known after apply)
+ limits = (known after apply)
+ main_script_base64 = (known after apply)
~ migration_tag = "v1" -> (known after apply)
~ modules = [
- { # forces replacement
- content_file = "../../..//packages/control-plane/dist/index.js" -> null
- content_sha256 = "c6feabd617fe14ecb501d6cf953e0b11e6e785a4fec6006bebce7a9891a2ee04" -> null
- content_type = "application/javascript+module" -> null
- name = "index.js" -> null
},
+ { # forces replacement
+ content_file = "../../..//packages/control-plane/dist/index.js"
+ content_sha256 = "717bc1123d997eca0a615c06ec1e22029fde019030e947b67faa1a829e4828f0"
+ content_type = "application/javascript+module"
+ name = "index.js"
},
]
~ number = 57 -> (known after apply)
~ source = "terraform" -> (known after apply)
~ startup_time_ms = 45 -> (known after apply)
~ urls = [] -> (known after apply)
# (6 unchanged attributes hidden)
}
# module.control_plane_worker.cloudflare_workers_deployment.this must be replaced
-/+ resource "cloudflare_workers_deployment" "this" {
~ annotations = {
+ workers_message = (known after apply)
~ workers_triggered_by = "deployment" -> (known after apply)
} -> (known after apply)
~ author_email = "alejo@primo.la" -> (known after apply)
~ created_on = "2026-07-26T06:18:36Z" -> (known after apply)
~ id = "6c613670-4aa5-41bd-bb95-206aee4ccd49" -> (known after apply)
~ source = "terraform" -> (known after apply)
~ versions = [ # forces replacement
~ {
~ version_id = "5a41bfd5-5d1b-4f9e-a282-9235a9ad3bd9" -> (known after apply)
# (1 unchanged attribute hidden)
},
]
# (3 unchanged attributes hidden)
}
# module.github_bot_worker[0].cloudflare_worker.this will be updated in-place
~ resource "cloudflare_worker" "this" {
id = "4b5e2696491a41eaaa124f4e2a9855f2"
name = "open-inspect-github-bot-primo"
~ observability = {
~ logs = {
+ destinations = (known after apply)
# (4 unchanged attributes hidden)
}
~ traces = {
+ destinations = (known after apply)
# (3 unchanged attributes hidden)
}
# (2 unchanged attributes hidden)
}
~ references = {
~ dispatch_namespace_outbounds = [] -> (known after apply)
~ domains = [] -> (known after apply)
~ durable_objects = [] -> (known after apply)
~ queues = [] -> (known after apply)
~ workers = [] -> (known after apply)
} -> (known after apply)
tags = []
~ updated_on = "2026-07-26T06:18:37Z" -> (known after apply)
# (6 unchanged attributes hidden)
}
# module.github_bot_worker[0].cloudflare_worker_version.this must be replaced
-/+ resource "cloudflare_worker_version" "this" {
~ annotations = {
+ workers_message = (known after apply)
+ workers_tag = (known after apply)
~ workers_triggered_by = "create_version_api" -> (known after apply)
} -> (known after apply)
~ bindings = (sensitive value) # forces replacement
~ created_on = "2026-07-26T06:18:37Z" -> (known after apply)
~ id = "7aaed149-c286-4edb-9ace-310ce4d3ccd1" -> (known after apply)
+ limits = (known after apply)
+ main_script_base64 = (known after apply)
+ migration_tag = (known after apply)
~ number = 41 -> (known after apply)
~ source = "terraform" -> (known after apply)
~ startup_time_ms = 35 -> (known after apply)
~ urls = [
- "https://7aaed149-open-inspect-github-bot-primo.primo-bf6.workers.dev",
] -> (known after apply)
# (7 unchanged attributes hidden)
}
# module.github_bot_worker[0].cloudflare_workers_deployment.this must be replaced
-/+ resource "cloudflare_workers_deployment" "this" {
~ annotations = {
+ workers_message = (known after apply)
~ workers_triggered_by = "deployment" -> (known after apply)
} -> (known after apply)
~ author_email = "alejo@primo.la" -> (known after apply)
~ created_on = "2026-07-26T06:18:38Z" -> (known after apply)
~ id = "2129eb15-5cf8-47a9-9f96-ea9e5ddae4e6" -> (known after apply)
~ source = "terraform" -> (known after apply)
~ versions = [ # forces replacement
~ {
~ version_id = "7aaed149-c286-4edb-9ace-310ce4d3ccd1" -> (known after apply)
# (1 unchanged attribute hidden)
},
]
# (3 unchanged attributes hidden)
}
# module.linear_bot_worker[0].cloudflare_worker.this will be updated in-place
~ resource "cloudflare_worker" "this" {
id = "33782d80e8ff4af9b30b92870084b674"
name = "open-inspect-linear-bot-primo"
~ observability = {
~ logs = {
+ destinations = (known after apply)
# (4 unchanged attributes hidden)
}
~ traces = {
+ destinations = (known after apply)
# (3 unchanged attributes hidden)
}
# (2 unchanged attributes hidden)
}
~ references = {
~ dispatch_namespace_outbounds = [] -> (known after apply)
~ domains = [] -> (known after apply)
~ durable_objects = [] -> (known after apply)
~ queues = [] -> (known after apply)
~ workers = [
- {
- id = "c208a60c393e45e38eb502346bb7ce1e" -> null
- name = "open-inspect-control-plane-primo" -> null
},
] -> (known after apply)
} -> (known after apply)
tags = []
~ updated_on = "2026-07-26T06:18:25Z" -> (known after apply)
# (6 unchanged attributes hidden)
}
# module.linear_bot_worker[0].cloudflare_worker_version.this must be replaced
-/+ resource "cloudflare_worker_version" "this" {
~ annotations = {
+ workers_message = (known after apply)
+ workers_tag = (known after apply)
~ workers_triggered_by = "create_version_api" -> (known after apply)
} -> (known after apply)
~ bindings = (sensitive value) # forces replacement
~ created_on = "2026-07-26T06:18:26Z" -> (known after apply)
~ id = "16b53281-fe2f-4885-a89e-c2239de3b811" -> (known after apply)
+ limits = (known after apply)
+ main_script_base64 = (known after apply)
+ migration_tag = (known after apply)
~ modules = [
- { # forces replacement
- content_file = "../../..//packages/linear-bot/dist/index.js" -> null
- content_sha256 = "2517fbd24658ee26edf46886075207f1ec4681a04695a102b050176899387c27" -> null
- content_type = "application/javascript+module" -> null
- name = "index.js" -> null
},
+ { # forces replacement
+ content_file = "../../..//packages/linear-bot/dist/index.js"
+ content_sha256 = "40fe0740664b29a528bebade6c0cdcb976f112ca86bf7d3f5040e1e82299b9ab"
+ content_type = "application/javascript+module"
+ name = "index.js"
},
]
~ number = 54 -> (known after apply)
~ source = "terraform" -> (known after apply)
~ startup_time_ms = 33 -> (known after apply)
~ urls = [
- "https://16b53281-open-inspect-linear-bot-primo.primo-bf6.workers.dev",
] -> (known after apply)
# (6 unchanged attributes hidden)
}
# module.linear_bot_worker[0].cloudflare_workers_deployment.this must be replaced
-/+ resource "cloudflare_workers_deployment" "this" {
~ annotations = {
+ workers_message = (known after apply)
~ workers_triggered_by = "deployment" -> (known after apply)
} -> (known after apply)
~ author_email = "alejo@primo.la" -> (known after apply)
~ created_on = "2026-07-26T06:18:27Z" -> (known after apply)
~ id = "5dc80780-d35d-46b2-8483-290d83a77ea2" -> (known after apply)
~ source = "terraform" -> (known after apply)
~ versions = [ # forces replacement
~ {
~ version_id = "16b53281-fe2f-4885-a89e-c2239de3b811" -> (known after apply)
# (1 unchanged attribute hidden)
},
]
# (3 unchanged attributes hidden)
}
# module.slack_bot_worker[0].cloudflare_worker.this will be updated in-place
~ resource "cloudflare_worker" "this" {
id = "5200e96d69804ea296e1f3a6b39e4243"
name = "open-inspect-slack-bot-primo"
~ observability = {
~ logs = {
+ destinations = (known after apply)
# (4 unchanged attributes hidden)
}
~ traces = {
+ destinations = (known after apply)
# (3 unchanged attributes hidden)
}
# (2 unchanged attributes hidden)
}
~ references = {
~ dispatch_namespace_outbounds = [] -> (known after apply)
~ domains = [] -> (known after apply)
~ durable_objects = [] -> (known after apply)
~ queues = [
- {
- queue_consumer_id = "a755a290fd92417fb11c298f9c1d1f40" -> null
- queue_id = "56ef0f3e13bd46a3a39f30c79ec547fa" -> null
- queue_name = "open-inspect-slack-completion-primo" -> null
},
] -> (known after apply)
~ workers = [
- {
- id = "c208a60c393e45e38eb502346bb7ce1e" -> null
- name = "open-inspect-control-plane-primo" -> null
},
] -> (known after apply)
} -> (known after apply)
tags = []
~ updated_on = "2026-07-26T06:18:25Z" -> (known after apply)
# (6 unchanged attributes hidden)
}
# module.slack_bot_worker[0].cloudflare_worker_version.this must be replaced
-/+ resource "cloudflare_worker_version" "this" {
~ annotations = {
+ workers_message = (known after apply)
+ workers_tag = (known after apply)
~ workers_triggered_by = "create_version_api" -> (known after apply)
} -> (known after apply)
~ bindings = (sensitive value) # forces replacement
~ created_on = "2026-07-26T06:18:26Z" -> (known after apply)
~ id = "6551b217-d1f4-4f1d-8072-253ad7a0d163" -> (known after apply)
+ limits = (known after apply)
+ main_script_base64 = (known after apply)
+ migration_tag = (known after apply)
~ number = 57 -> (known after apply)
~ source = "terraform" -> (known after apply)
~ startup_time_ms = 36 -> (known after apply)
~ urls = [
- "https://6551b217-open-inspect-slack-bot-primo.primo-bf6.workers.dev",
] -> (known after apply)
# (7 unchanged attributes hidden)
}
# module.slack_bot_worker[0].cloudflare_workers_deployment.this must be replaced
-/+ resource "cloudflare_workers_deployment" "this" {
~ annotations = {
+ workers_message = (known after apply)
~ workers_triggered_by = "deployment" -> (known after apply)
} -> (known after apply)
~ author_email = "alejo@primo.la" -> (known after apply)
~ created_on = "2026-07-26T06:18:27Z" -> (known after apply)
~ id = "21642fd7-f856-48de-a26d-d501d8817102" -> (known after apply)
~ source = "terraform" -> (known after apply)
~ versions = [ # forces replacement
~ {
~ version_id = "6551b217-d1f4-4f1d-8072-253ad7a0d163" -> (known after apply)
# (1 unchanged attribute hidden)
},
]
# (3 unchanged attributes hidden)
}
Plan: 17 to add, 4 to change, 16 to destroy.
─────────────────────────────────────────────────────────────────────────────
Saved the plan to: tfplan
To perform exactly these actions, run the following command to apply:
terraform apply "tfplan"Pushed by: @NicolasWalter |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: d61fb5f68f
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
Comment on lines
+129
to
+135
| return { | ||
| scmUserId: profile.data.subject, | ||
| scmLogin: profile.data.login, | ||
| displayName: profile.data.displayName ?? profile.data.login, | ||
| email: author?.email, | ||
| accessTokenEncrypted, | ||
| ...(token.accessTokenExpiresAt ? { tokenExpiresAt: token.accessTokenExpiresAt.getTime() } : {}), |
There was a problem hiding this comment.
When GitHub issues an expiring user token, this enrichment copies the access token and expiry into the session but deliberately omits the refresh token without providing another refresh path. After the expiry—such as when a long-running or resumed session creates a PR before another browser prompt refreshes its participant—the existing ParticipantService.refreshTokenLocal() finds no scm_refresh_token_encrypted, and resolveAuthForPR() falls back to the GitHub App token, losing the user's credentials and attribution. Keep Better Auth as the token authority, but make expired session credentials refresh through it (or otherwise preserve a usable refresh capability).
Useful? React with 👍 / 👎.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 12
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
packages/control-plane/src/routes/session-create.ts (1)
144-167: 🔒 Security & Privacy | 🟠 Major | ⚡ Quick winFail-closed authority resolution is swallowed by best-effort enrichment catches in both route handlers.
resolveGitHubCredentialAuthoritythrows on missing browser-session provenance, an unavailable auth runtime, or corrupt/ambiguous account state. Both call sites evaluate it inside thetrywhosecatchmerely warns and continues, converting deliberate integrity failures into a silent degrade to the bot fallback. The genuinely optional case (no linked GitHub account) already returnsnullwithout throwing, so it does not depend on this catch.
packages/control-plane/src/routes/session-create.ts#L144-L167: hoist theawait resolveGitHubCredentialAuthority(ctx, request.headers)call above thetry(or rethrow non-enrichment errors) so authority failures produce an error response instead of a warning.packages/control-plane/src/routes/session-prompt.ts#L93-L100: apply the same hoist so prompt enrichment does not mask provenance/integrity failures.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/control-plane/src/routes/session-create.ts` around lines 144 - 167, Move the resolveGitHubCredentialAuthority call outside the best-effort enrichment try/catch in session-create.ts (lines 144-167), and apply the same change in session-prompt.ts (lines 93-100). Pass the resolved authority into resolveGitHubEnrichmentForRequest so authority/provenance failures propagate as error responses, while the catch continues handling only optional enrichment failures.
🧹 Nitpick comments (14)
packages/linear-bot/src/utils/integration-config.test.ts (1)
81-89: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winReuse the named fallback configuration.
Export/import and assert against
DEFAULT_CONFIGrather than repeating its literal values in both fallback tests. As per coding guidelines, “Define each default value exactly once in a named constant and import or reuse that constant everywhere.”Also applies to: 103-111
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/linear-bot/src/utils/integration-config.test.ts` around lines 81 - 89, Update the fallback tests around the configuration resolution assertions to import and compare against the named DEFAULT_CONFIG constant instead of duplicating its literal values. Apply this to both fallback test cases, preserving the existing resolve behavior while ensuring defaults are defined and validated in one place.Source: Coding guidelines
packages/control-plane/wrangler.jsonc (1)
5-6: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winCompatibility date and flags are declared in two places and must match Terraform. The test worker runtime is now configured independently in both files, so they can drift from each other and from the Terraform-managed production worker.
packages/control-plane/wrangler.jsonc#L5-L6: confirm2024-09-23+nodejs_compatmatchesworkers-control-plane.tf; the date moves backward from2024-12-30.packages/control-plane/vitest.integration.config.ts#L48-L51: derivecompatibilityDate/compatibilityFlagsfrom the loadedwrangler.jsoncinstead of re-hardcoding them.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/control-plane/wrangler.jsonc` around lines 5 - 6, Align packages/control-plane/wrangler.jsonc lines 5-6 with workers-control-plane.tf using compatibility date 2024-09-23 and the nodejs_compat flag. Update packages/control-plane/vitest.integration.config.ts lines 48-51 to load compatibilityDate and compatibilityFlags from the parsed wrangler.jsonc configuration instead of hard-coding them, keeping both test and production runtime settings synchronized.Source: Coding guidelines
packages/control-plane/test/integration/helpers.ts (1)
37-44: 🚀 Performance & Scalability | 🔵 Trivial | 💤 Low valueSeeding on every
webrequest adds four D1 writes per call.
testBrowserSessionCookie()re-runs the batch for eachserviceFetch. Consider memoizing the cookie per test file (reset alongsidecleanD1Tables()) to cut integration-test I/O.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/control-plane/test/integration/helpers.ts` around lines 37 - 44, The testBrowserSessionCookie function currently performs its D1 seeding batch on every serviceFetch. Memoize the generated cookie for the test file so repeated calls reuse it without rerunning env.DB.batch, and clear that cached value alongside cleanD1Tables() so each test starts with fresh state.packages/control-plane/vitest.integration.config.ts (1)
85-105: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winStack-path match on
/better-auth/dist/api/routes/is brittle across upgrades.If the package layout changes on a
better-authbump, redirect rejections stop matching and integration suites fail as unhandled errors. Consider matching onname === "APIError"+ 3xx status alone, or asserting onlybetter-authin the stack.♻️ Looser match
- betterAuthStack?.includes("/better-auth/dist/api/routes/") + betterAuthStack?.includes("better-auth")🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/control-plane/vitest.integration.config.ts` around lines 85 - 105, Loosen the filtering condition in onUnhandledError so Better Auth redirect control flow remains recognized across package layout changes. Keep the APIError name and 3xx status checks, but replace the brittle "/better-auth/dist/api/routes/" stack-path requirement with a stable better-auth stack check, or rely on those APIError/3xx checks alone; continue returning false only for the intended redirects.packages/web/src/app/api/sessions/[id]/attachments/[attachmentId]/route.ts (1)
45-50: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueExtract the shared passthrough-header allowlist. Both binary proxies now hardcode the same
["Content-Type", "Content-Range", "Accept-Ranges", "ETag"]list and copy loop, so any future header-policy change (like thisContent-Lengthremoval) has to be made twice.
packages/web/src/app/api/sessions/[id]/attachments/[attachmentId]/route.ts#L45-L50: import a shared constant/helper instead of the inline array and loop.packages/web/src/app/api/sessions/[id]/media/[artifactId]/route.ts#L42-L47: reuse the same shared constant/helper.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/web/src/app/api/sessions/`[id]/attachments/[attachmentId]/route.ts around lines 45 - 50, Extract the shared passthrough-header allowlist and copying logic from the binary proxy routes into a reusable constant or helper. Update packages/web/src/app/api/sessions/[id]/attachments/[attachmentId]/route.ts lines 45-50 and packages/web/src/app/api/sessions/[id]/media/[artifactId]/route.ts lines 42-47 to reuse it, preserving the current Content-Type, Content-Range, Accept-Ranges, and ETag policy.terraform/d1/migrations/0048_better_auth_core.sql (1)
22-35: 🚀 Performance & Scalability | 🔵 TrivialConsider an index on
auth_sessions(expiresAt)for expiry sweeps.Better Auth's session cleanup / expired-session deletion scans by
expiresAt; without an index that is a full table scan as sessions accumulate.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@terraform/d1/migrations/0048_better_auth_core.sql` around lines 22 - 35, Update the auth_sessions schema migration to add an index on the expiresAt column for expiry cleanup queries. Define the index alongside auth_sessions_userId_idx, preserving the existing table and userId index definitions.packages/control-plane/src/auth/service/request-authenticator.ts (1)
39-71: 🔒 Security & Privacy | 🔵 Trivial | 🏗️ Heavy liftNonce reuse is detected but not rejected.
recordNoncelogs and returns on a hit, so a captured signature can be replayed withinTOKEN_VALIDITY_MS. The comment says "log-only for now" — worth tracking as a follow-up to enforce rejection (in-isolate state makes this best-effort; a durable store would be needed for a real guarantee).🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/control-plane/src/auth/service/request-authenticator.ts` around lines 39 - 71, Track nonce reuse enforcement as a follow-up from recordNonce: preserve the current log-only behavior for now, but add a clear TODO or issue reference documenting that reused nonces must eventually be rejected within TOKEN_VALIDITY_MS, acknowledging the best-effort in-isolate limitation.packages/control-plane/src/auth/authenticate.test.ts (1)
312-374: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winConsider covering the two 500 paths.
authenticatealso returns 500 whenctx.getUserAuthis absent and whenauthenticateSessionthrows (SessionIntegrityErrorvs runtime). Neither branch is exercised here, and both are security-relevant fail-closed paths.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/control-plane/src/auth/authenticate.test.ts` around lines 312 - 374, Extend the compound browser credentials tests around createUserAuthContext and authenticate to cover both 500 fail-closed paths: a context without ctx.getUserAuth, and an authenticateSession failure that distinguishes SessionIntegrityError from an unexpected runtime error. Assert each branch’s status and error response, while preserving the existing successful and absent-session coverage.packages/control-plane/src/source-control/github-credential-authority.test.ts (1)
56-135: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winAdd cases for the remaining throw paths.
Uncovered branches in
resolveGitHubCredentialAuthority: schema-failure on a malformedlistUserAccountspayload (same "corrupt" message but a different cause), missinggetUserAuth, and a non-user principal carryingauthentication.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/control-plane/src/source-control/github-credential-authority.test.ts` around lines 56 - 135, Add tests for the remaining throw branches in resolveGitHubCredentialAuthority: make listUserAccounts return a malformed payload and assert the “GitHub account authority is corrupt” error, omit getUserAuth and assert its expected failure, and provide a non-user principal with authentication to verify that path rejects appropriately. Reuse the existing createContext/createUserContext helpers and preserve the current assertions.packages/control-plane/src/auth/user/session-authenticator.test.ts (1)
1-49: 🎯 Functional Correctness | 🔵 Trivial | ⚡ Quick winAdd a test for the malformed-session branch.
Coverage exists for null and cross-user sessions, but not for
sessionSchema.safeParsefailure (e.g., missingsession.id), which throwsSessionIntegrityError("Better Auth returned a malformed session")insession-authenticator.ts. Consider adding a case asserting this throw for a candidate missing required fields.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/control-plane/src/auth/user/session-authenticator.test.ts` around lines 1 - 49, Add a test in the authenticateSession suite for a Better Auth result whose session omits a required field such as id, and assert that authenticateSession rejects with “Better Auth returned a malformed session” (SessionIntegrityError). Keep the existing null and cross-user cases unchanged.packages/control-plane/src/auth/user/providers/github-identity.ts (1)
6-11: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winGitHub API base URL/version/user-agent are duplicated instead of centralized.
github-identity.tsdefines local, unexportedGITHUB_API_URL/GITHUB_API_VERSIONconstants, andadmission-policy.tsindependently hardcodes the same base URL and API version string plus its own literal user-agent — one root cause: no shared constants module for these GitHub API literals (unlikeDEFAULT_PROVIDER_REQUEST_TIMEOUT_MS, which is already correctly centralized inproviders/constants.ts).
packages/control-plane/src/auth/user/providers/github-identity.ts#L6-L11: exportGITHUB_API_URL,GITHUB_API_VERSION(and a shared default user-agent) fromproviders/constants.tsinstead of keeping them file-local.packages/control-plane/src/auth/user/admission-policy.ts#L130-L138: import and reuse the shared constants instead of re-hardcoding"https://api.github.com/user/memberships/orgs/","2022-11-28", and"Open-Inspect-Control-Plane".As per coding guidelines, "Define each default value exactly once in a named constant and import or reuse that constant everywhere."
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/control-plane/src/auth/user/providers/github-identity.ts` around lines 6 - 11, The GitHub API URL, API version, and default user-agent are duplicated instead of centralized. In packages/control-plane/src/auth/user/providers/github-identity.ts lines 6-11, move or export these named constants through providers/constants.ts; in packages/control-plane/src/auth/user/admission-policy.ts lines 130-138, import and reuse them for the memberships URL, API version, and user-agent instead of literals.Source: Coding guidelines
packages/control-plane/src/routes/browser-auth.test.ts (1)
4-27: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winConsider covering the non-session branch and the
service:webgate.Only the
get-sessionshort-circuit is exercised. Theauth.handler(request)fallback andrequireWebServicerejecting non-service:webprincipals are the security-relevant paths and currently untested here.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/control-plane/src/routes/browser-auth.test.ts` around lines 4 - 27, Add tests alongside the existing forwardBrowserAuthRequest test to cover the non-session request path invoking auth.handler(request), and the requireWebService gate rejecting principals without the service:web role. Reuse the visible auth/request setup patterns and assert the fallback response plus the rejection behavior.packages/control-plane/src/types.ts (1)
100-104: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winDocument the admission-policy variables, especially
UNSAFE_ALLOW_ALL_USERS.Every neighbouring entry carries a trailing comment describing its format and effect. These five drive who may sign in, and
UNSAFE_ALLOW_ALL_USERSdisables the allowlist entirely — note the expected format (comma-separated?) and the security implication inline.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/control-plane/src/types.ts` around lines 100 - 104, The admission-policy entries in the environment-variable type definition lack inline documentation. Add trailing comments to ALLOWED_USERS, ALLOWED_EMAIL_DOMAINS, ALLOWED_EMAILS, ALLOWED_GITHUB_ORGS, and UNSAFE_ALLOW_ALL_USERS describing each expected format and access effect; explicitly document that UNSAFE_ALLOW_ALL_USERS disables the allowlist and its security implication.packages/control-plane/src/routes/browser-auth.ts (1)
36-42: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winExtract the session path into a shared constant.
/api/auth/get-sessionis duplicated here and inBROWSER_AUTH_PROXY_ROUTES; centralizing it avoids drift if the auth path changes.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/control-plane/src/routes/browser-auth.ts` around lines 36 - 42, Define a shared constant for the "/api/auth/get-session" path and use it in the GET condition within the browser-auth request handler and in BROWSER_AUTH_PROXY_ROUTES. Remove the duplicated string literals while preserving the existing routing behavior.Source: Coding guidelines
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@packages/control-plane/src/auth/user/better-auth.ts`:
- Around line 66-76: Update the GitHub provider configuration in the
socialProviders setup to retain disableDefaultScope: true while explicitly
supplying scopes for read:user and user:email. Preserve any values from
config.github and ensure the explicit scope array is applied so GitHub sign-in
can resolve the user profile and email.
- Around line 45-50: Update the Better Auth rateLimit configuration to use
database-backed storage instead of memory, and add the corresponding Better Auth
rate-limit migration/table required by the database adapter. Preserve the
existing enabled, window, and max settings.
In `@packages/control-plane/src/db/browser-auth-legacy-migration.test.ts`:
- Around line 1-19: Update the repository’s Node.js version constraint to
require >=22.5.0, locating the package/toolchain configuration that defines the
supported Node 22 range. Keep the existing version policy unchanged apart from
raising the minimum needed by the node:sqlite import in this test.
In `@packages/control-plane/src/session/identity.ts`:
- Around line 66-79: Update browser GitHub account matching to compare
account.subject with browserGitHubAccountInfoSchema’s data.subject, not user.id;
retain user.id exclusively as the Better Auth user record identifier.
In `@packages/control-plane/src/worker-build.test.ts`:
- Around line 14-21: Add timeout: WORKER_BUILD_TIMEOUT_MS to both execFileSync
calls in the worker build test, covering the shared-package and worker-package
npm builds while preserving their existing cwd and stdio options.
In `@packages/control-plane/test/integration/browser-auth.test.ts`:
- Around line 1-15: Register shared-D1 cleanup for both integration suites: in
packages/control-plane/test/integration/browser-auth.test.ts lines 1-15 and
packages/control-plane/test/integration/browser-auth-router.test.ts lines 1-10,
import cleanD1Tables from ./cleanup and add beforeEach(cleanD1Tables) before the
describe blocks.
In `@packages/control-plane/test/integration/helpers.ts`:
- Around line 15-28: Update signCookieValue to encode the HMAC signature as
unpadded base64url instead of standard base64 from btoa. Convert the signature
bytes to base64, replace + and / with - and _, remove trailing padding, and
retain the existing cookie value encoding and signing flow.
In `@packages/web/src/lib/browser-auth-proxy.ts`:
- Around line 55-60: Update trustedClientIp to obtain Cloudflare metadata
through getCloudflareContext() instead of checking for a cf property on the
Request. Preserve the Vercel X-Vercel-Forwarded-For behavior, and read
CF-Connecting-IP from the Cloudflare context for non-Vercel deployments.
In `@packages/web/src/lib/control-plane-transport.ts`:
- Around line 90-106: The unconditional timeout in the transport path also
aborts streamed response bodies after headers are received. Update the
fetch-options flow around boundedFetchOptions and the control-plane callers that
proxy attachment/media/diff streams so streaming requests can explicitly skip
the internal CONTROL_PLANE_FETCH_TIMEOUT_MS signal, while retaining the timeout
by default for normal requests and preserving caller-provided signal handling.
In `@packages/web/src/lib/server-auth-session.ts`:
- Around line 28-47: Update getServerAuthSession to contain failures from
serializeBrowserSessionCookies, the non-success dispatchBrowserAuthRequest
response, and browserAuthSessionResponseSchema.parse so callers receive the
intended unauthenticated result or a consistently handled authentication error
instead of an unhandled exception. Prefer returning null for malformed cookies
and invalid session payloads, and ensure any deliberately thrown error is
handled consistently by all consumers.
In `@terraform/d1/migrations/0049_backfill_better_auth_accounts.sql`:
- Around line 21-46: Add an ON CONFLICT(id) DO NOTHING clause to the INSERT into
auth_users in the backfill migration, preserving the existing SELECT and NOT
EXISTS filtering while preventing duplicate primary-key rows from aborting the
migration.
In `@terraform/environments/production/.terraform.lock.hcl`:
- Around line 88-103: Update the hashicorp/random lock entry’s hashes list to
include the complete platform coverage for linux_amd64 and darwin_arm64,
regenerating it with terraform providers lock and preserving the existing
checksums alongside the additional h1 hash.
---
Outside diff comments:
In `@packages/control-plane/src/routes/session-create.ts`:
- Around line 144-167: Move the resolveGitHubCredentialAuthority call outside
the best-effort enrichment try/catch in session-create.ts (lines 144-167), and
apply the same change in session-prompt.ts (lines 93-100). Pass the resolved
authority into resolveGitHubEnrichmentForRequest so authority/provenance
failures propagate as error responses, while the catch continues handling only
optional enrichment failures.
---
Nitpick comments:
In `@packages/control-plane/src/auth/authenticate.test.ts`:
- Around line 312-374: Extend the compound browser credentials tests around
createUserAuthContext and authenticate to cover both 500 fail-closed paths: a
context without ctx.getUserAuth, and an authenticateSession failure that
distinguishes SessionIntegrityError from an unexpected runtime error. Assert
each branch’s status and error response, while preserving the existing
successful and absent-session coverage.
In `@packages/control-plane/src/auth/service/request-authenticator.ts`:
- Around line 39-71: Track nonce reuse enforcement as a follow-up from
recordNonce: preserve the current log-only behavior for now, but add a clear
TODO or issue reference documenting that reused nonces must eventually be
rejected within TOKEN_VALIDITY_MS, acknowledging the best-effort in-isolate
limitation.
In `@packages/control-plane/src/auth/user/providers/github-identity.ts`:
- Around line 6-11: The GitHub API URL, API version, and default user-agent are
duplicated instead of centralized. In
packages/control-plane/src/auth/user/providers/github-identity.ts lines 6-11,
move or export these named constants through providers/constants.ts; in
packages/control-plane/src/auth/user/admission-policy.ts lines 130-138, import
and reuse them for the memberships URL, API version, and user-agent instead of
literals.
In `@packages/control-plane/src/auth/user/session-authenticator.test.ts`:
- Around line 1-49: Add a test in the authenticateSession suite for a Better
Auth result whose session omits a required field such as id, and assert that
authenticateSession rejects with “Better Auth returned a malformed session”
(SessionIntegrityError). Keep the existing null and cross-user cases unchanged.
In `@packages/control-plane/src/routes/browser-auth.test.ts`:
- Around line 4-27: Add tests alongside the existing forwardBrowserAuthRequest
test to cover the non-session request path invoking auth.handler(request), and
the requireWebService gate rejecting principals without the service:web role.
Reuse the visible auth/request setup patterns and assert the fallback response
plus the rejection behavior.
In `@packages/control-plane/src/routes/browser-auth.ts`:
- Around line 36-42: Define a shared constant for the "/api/auth/get-session"
path and use it in the GET condition within the browser-auth request handler and
in BROWSER_AUTH_PROXY_ROUTES. Remove the duplicated string literals while
preserving the existing routing behavior.
In
`@packages/control-plane/src/source-control/github-credential-authority.test.ts`:
- Around line 56-135: Add tests for the remaining throw branches in
resolveGitHubCredentialAuthority: make listUserAccounts return a malformed
payload and assert the “GitHub account authority is corrupt” error, omit
getUserAuth and assert its expected failure, and provide a non-user principal
with authentication to verify that path rejects appropriately. Reuse the
existing createContext/createUserContext helpers and preserve the current
assertions.
In `@packages/control-plane/src/types.ts`:
- Around line 100-104: The admission-policy entries in the environment-variable
type definition lack inline documentation. Add trailing comments to
ALLOWED_USERS, ALLOWED_EMAIL_DOMAINS, ALLOWED_EMAILS, ALLOWED_GITHUB_ORGS, and
UNSAFE_ALLOW_ALL_USERS describing each expected format and access effect;
explicitly document that UNSAFE_ALLOW_ALL_USERS disables the allowlist and its
security implication.
In `@packages/control-plane/test/integration/helpers.ts`:
- Around line 37-44: The testBrowserSessionCookie function currently performs
its D1 seeding batch on every serviceFetch. Memoize the generated cookie for the
test file so repeated calls reuse it without rerunning env.DB.batch, and clear
that cached value alongside cleanD1Tables() so each test starts with fresh
state.
In `@packages/control-plane/vitest.integration.config.ts`:
- Around line 85-105: Loosen the filtering condition in onUnhandledError so
Better Auth redirect control flow remains recognized across package layout
changes. Keep the APIError name and 3xx status checks, but replace the brittle
"/better-auth/dist/api/routes/" stack-path requirement with a stable better-auth
stack check, or rely on those APIError/3xx checks alone; continue returning
false only for the intended redirects.
In `@packages/control-plane/wrangler.jsonc`:
- Around line 5-6: Align packages/control-plane/wrangler.jsonc lines 5-6 with
workers-control-plane.tf using compatibility date 2024-09-23 and the
nodejs_compat flag. Update packages/control-plane/vitest.integration.config.ts
lines 48-51 to load compatibilityDate and compatibilityFlags from the parsed
wrangler.jsonc configuration instead of hard-coding them, keeping both test and
production runtime settings synchronized.
In `@packages/linear-bot/src/utils/integration-config.test.ts`:
- Around line 81-89: Update the fallback tests around the configuration
resolution assertions to import and compare against the named DEFAULT_CONFIG
constant instead of duplicating its literal values. Apply this to both fallback
test cases, preserving the existing resolve behavior while ensuring defaults are
defined and validated in one place.
In `@packages/web/src/app/api/sessions/`[id]/attachments/[attachmentId]/route.ts:
- Around line 45-50: Extract the shared passthrough-header allowlist and copying
logic from the binary proxy routes into a reusable constant or helper. Update
packages/web/src/app/api/sessions/[id]/attachments/[attachmentId]/route.ts lines
45-50 and packages/web/src/app/api/sessions/[id]/media/[artifactId]/route.ts
lines 42-47 to reuse it, preserving the current Content-Type, Content-Range,
Accept-Ranges, and ETag policy.
In `@terraform/d1/migrations/0048_better_auth_core.sql`:
- Around line 22-35: Update the auth_sessions schema migration to add an index
on the expiresAt column for expiry cleanup queries. Define the index alongside
auth_sessions_userId_idx, preserving the existing table and userId index
definitions.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: c2cbb6c1-914f-41e2-9120-b14ee6ea7adc
⛔ Files ignored due to path filters (1)
package-lock.jsonis excluded by!**/package-lock.json
📒 Files selected for processing (191)
.github/workflows/ci.yml.github/workflows/deploy-web.ymldocs/GETTING_STARTED.mddocs/SETUP_GUIDE.mdeslint.config.jspackages/control-plane/README.mdpackages/control-plane/package.jsonpackages/control-plane/src/auth/auth-encryption.test.tspackages/control-plane/src/auth/auth-encryption.tspackages/control-plane/src/auth/authenticate.test.tspackages/control-plane/src/auth/authenticate.tspackages/control-plane/src/auth/identity-enforcement.test.tspackages/control-plane/src/auth/identity-enforcement.tspackages/control-plane/src/auth/index.tspackages/control-plane/src/auth/oauth-authorization-service.test.tspackages/control-plane/src/auth/oauth-authorization-service.tspackages/control-plane/src/auth/oauth-flow-state.tspackages/control-plane/src/auth/oauth-flow-verifier.tspackages/control-plane/src/auth/pkce.test.tspackages/control-plane/src/auth/pkce.tspackages/control-plane/src/auth/principal.tspackages/control-plane/src/auth/provider-credential-cipher.tspackages/control-plane/src/auth/providers/github.test.tspackages/control-plane/src/auth/providers/github.tspackages/control-plane/src/auth/providers/google.test.tspackages/control-plane/src/auth/providers/google.tspackages/control-plane/src/auth/providers/oidc-authorization-code-client.tspackages/control-plane/src/auth/providers/types.test.tspackages/control-plane/src/auth/providers/types.tspackages/control-plane/src/auth/result.tspackages/control-plane/src/auth/service/callback-signing.tspackages/control-plane/src/auth/service/config.tspackages/control-plane/src/auth/service/request-authenticator.tspackages/control-plane/src/auth/subject-verification.test.tspackages/control-plane/src/auth/subject-verification.tspackages/control-plane/src/auth/token-exchange.tspackages/control-plane/src/auth/user/admission-policy.test.tspackages/control-plane/src/auth/user/admission-policy.tspackages/control-plane/src/auth/user/better-auth.tspackages/control-plane/src/auth/user/canonical-user-projection.tspackages/control-plane/src/auth/user/provider-credential.tspackages/control-plane/src/auth/user/provider-profile.tspackages/control-plane/src/auth/user/providers/constants.tspackages/control-plane/src/auth/user/providers/github-identity.test.tspackages/control-plane/src/auth/user/providers/github-identity.tspackages/control-plane/src/auth/user/providers/github-profile.test.tspackages/control-plane/src/auth/user/providers/github-profile.tspackages/control-plane/src/auth/user/providers/google-profile.test.tspackages/control-plane/src/auth/user/providers/google-profile.tspackages/control-plane/src/auth/user/providers/types.tspackages/control-plane/src/auth/user/runtime.test.tspackages/control-plane/src/auth/user/runtime.tspackages/control-plane/src/auth/user/session-authenticator.test.tspackages/control-plane/src/auth/user/session-authenticator.tspackages/control-plane/src/auth/user/sign-in-provider.test.tspackages/control-plane/src/auth/user/sign-in-provider.tspackages/control-plane/src/auth/web-session-tokens.test.tspackages/control-plane/src/auth/web-session-tokens.tspackages/control-plane/src/db/api-tokens.test.tspackages/control-plane/src/db/api-tokens.tspackages/control-plane/src/db/browser-auth-legacy-migration.test.tspackages/control-plane/src/db/browser-auth-sessions.tspackages/control-plane/src/db/canonical-user-projection.tspackages/control-plane/src/db/errors.tspackages/control-plane/src/db/oauth-authorization-codes.tspackages/control-plane/src/db/oauth-flow-state.tspackages/control-plane/src/db/provider-credentials.tspackages/control-plane/src/router.analytics.test.tspackages/control-plane/src/router.auth.test.tspackages/control-plane/src/router.create-session.test.tspackages/control-plane/src/router.provider-identities.test.tspackages/control-plane/src/router.scm-credentials.test.tspackages/control-plane/src/router.session-prompt.test.tspackages/control-plane/src/router.spawn-child.test.tspackages/control-plane/src/router.tspackages/control-plane/src/routes/auth-tokens.tspackages/control-plane/src/routes/automations.test.tspackages/control-plane/src/routes/browser-auth.test.tspackages/control-plane/src/routes/browser-auth.tspackages/control-plane/src/routes/provider-identities.test.tspackages/control-plane/src/routes/provider-identities.tspackages/control-plane/src/routes/session-create.tspackages/control-plane/src/routes/session-prompt.tspackages/control-plane/src/routes/session-runtime-proxy.test.tspackages/control-plane/src/routes/shared.tspackages/control-plane/src/scheduler/durable-object.tspackages/control-plane/src/session/callback-notification-service.tspackages/control-plane/src/session/identity.test.tspackages/control-plane/src/session/identity.tspackages/control-plane/src/source-control/github-credential-authority.test.tspackages/control-plane/src/source-control/github-credential-authority.tspackages/control-plane/src/types.tspackages/control-plane/src/types/error.d.tspackages/control-plane/src/worker-build.test.tspackages/control-plane/test/integration/auth-tokens.test.tspackages/control-plane/test/integration/auth.test.tspackages/control-plane/test/integration/browser-auth-callback.test.tspackages/control-plane/test/integration/browser-auth-router.test.tspackages/control-plane/test/integration/browser-auth-sessions.test.tspackages/control-plane/test/integration/browser-auth.test.tspackages/control-plane/test/integration/cleanup.tspackages/control-plane/test/integration/helpers.tspackages/control-plane/test/integration/oauth-authorization-codes.test.tspackages/control-plane/test/integration/oauth-flow-state.test.tspackages/control-plane/test/integration/provider-credentials.test.tspackages/control-plane/test/integration/service-auth.test.tspackages/control-plane/tsconfig.test.jsonpackages/control-plane/vitest.integration.config.tspackages/control-plane/wrangler.jsoncpackages/linear-bot/src/classifier/index.test.tspackages/linear-bot/src/classifier/index.tspackages/linear-bot/src/utils/integration-config.test.tspackages/linear-bot/src/utils/integration-config.tspackages/linear-bot/src/webhook-handler.test.tspackages/linear-bot/src/webhook-handler.tspackages/shared/src/browser-auth-routes.test.tspackages/shared/src/browser-auth-routes.tspackages/shared/src/index.tspackages/web/.env.examplepackages/web/README.mdpackages/web/package.jsonpackages/web/src/app/access-denied/page.tsxpackages/web/src/app/api/auth/[...auth]/route.test.tspackages/web/src/app/api/auth/[...auth]/route.tspackages/web/src/app/api/auth/[...nextauth]/route.tspackages/web/src/app/api/auth/oi-refresh/route.test.tspackages/web/src/app/api/auth/oi-refresh/route.tspackages/web/src/app/api/automations/route.test.tspackages/web/src/app/api/automations/route.tspackages/web/src/app/api/sessions/[id]/attachments/[attachmentId]/route.test.tspackages/web/src/app/api/sessions/[id]/attachments/[attachmentId]/route.tspackages/web/src/app/api/sessions/[id]/diff/[revisionId]/files/[fileId]/route.test.tspackages/web/src/app/api/sessions/[id]/diff/[revisionId]/files/[fileId]/route.tspackages/web/src/app/api/sessions/[id]/diff/retry/route.tspackages/web/src/app/api/sessions/[id]/media/[artifactId]/route.test.tspackages/web/src/app/api/sessions/[id]/media/[artifactId]/route.tspackages/web/src/app/api/sessions/[id]/ws-token/route.test.tspackages/web/src/app/api/sessions/[id]/ws-token/route.tspackages/web/src/app/api/sessions/route.test.tspackages/web/src/app/api/sessions/route.tspackages/web/src/app/providers.test.tsxpackages/web/src/app/providers.tsxpackages/web/src/components/sidebar-layout.test.tsxpackages/web/src/components/web-session-gate.integration.test.tsxpackages/web/src/components/web-session-gate.test.tsxpackages/web/src/components/web-session-gate.tsxpackages/web/src/lib/access-control.test.tspackages/web/src/lib/access-control.tspackages/web/src/lib/auth-session.test.tsxpackages/web/src/lib/auth-session.tsxpackages/web/src/lib/auth.test.tspackages/web/src/lib/auth.tspackages/web/src/lib/browser-auth-proxy.test.tspackages/web/src/lib/browser-auth-proxy.tspackages/web/src/lib/browser-auth-session-contract.tspackages/web/src/lib/browser-session-cookie.test.tspackages/web/src/lib/browser-session-cookie.tspackages/web/src/lib/build-auth-identity.test.tspackages/web/src/lib/build-auth-identity.tspackages/web/src/lib/client-auth-boundary-eslint.test.tspackages/web/src/lib/control-plane-transport.test.tspackages/web/src/lib/control-plane-transport.tspackages/web/src/lib/control-plane.test.tspackages/web/src/lib/control-plane.tspackages/web/src/lib/current-user.test.tspackages/web/src/lib/current-user.tspackages/web/src/lib/github-email-schema.test.tspackages/web/src/lib/github-email-schema.tspackages/web/src/lib/github-org-membership.test.tspackages/web/src/lib/github-org-membership.tspackages/web/src/lib/oi-session.test.tspackages/web/src/lib/oi-session.tspackages/web/src/lib/server-auth-boundary-eslint.test.tspackages/web/src/lib/server-auth-session.test.tspackages/web/src/lib/server-auth-session.tspackages/web/src/lib/session-cookie.test.tspackages/web/src/lib/session-cookie.tspackages/web/src/lib/site-config.tsscripts/wrangler-secrets.shterraform/README.mdterraform/d1/migrations/0048_better_auth_core.sqlterraform/d1/migrations/0049_backfill_better_auth_accounts.sqlterraform/d1/migrations/0050_purge_retired_api_tokens.sqlterraform/environments/production/.terraform.lock.hclterraform/environments/production/checks.tfterraform/environments/production/locals.tfterraform/environments/production/terraform.tfvars.exampleterraform/environments/production/variables.tfterraform/environments/production/web-cloudflare.tfterraform/environments/production/web-vercel.tfterraform/environments/production/workers-control-plane.tf
💤 Files with no reviewable changes (50)
- packages/web/src/lib/auth.ts
- packages/web/src/lib/control-plane-transport.test.ts
- packages/web/src/app/api/auth/[...nextauth]/route.ts
- packages/control-plane/src/auth/providers/google.test.ts
- packages/control-plane/test/integration/oauth-flow-state.test.ts
- packages/control-plane/src/auth/index.ts
- packages/web/src/app/api/auth/oi-refresh/route.ts
- packages/web/src/app/api/auth/oi-refresh/route.test.ts
- packages/control-plane/src/auth/subject-verification.test.ts
- packages/control-plane/src/auth/providers/google.ts
- packages/web/src/components/web-session-gate.tsx
- packages/control-plane/src/auth/pkce.test.ts
- packages/control-plane/src/db/api-tokens.test.ts
- packages/control-plane/src/routes/provider-identities.test.ts
- packages/control-plane/src/auth/oauth-flow-verifier.ts
- packages/control-plane/src/auth/providers/github.ts
- packages/control-plane/src/auth/provider-credential-cipher.ts
- packages/control-plane/src/auth/oauth-authorization-service.ts
- packages/control-plane/src/router.provider-identities.test.ts
- packages/control-plane/src/auth/web-session-tokens.test.ts
- packages/control-plane/src/auth/auth-encryption.test.ts
- packages/control-plane/test/integration/oauth-authorization-codes.test.ts
- packages/control-plane/test/integration/browser-auth-sessions.test.ts
- packages/web/src/lib/auth.test.ts
- packages/control-plane/src/routes/provider-identities.ts
- packages/control-plane/src/db/oauth-flow-state.ts
- packages/control-plane/src/auth/providers/types.test.ts
- packages/web/src/lib/access-control.ts
- packages/control-plane/src/auth/pkce.ts
- packages/control-plane/src/auth/providers/github.test.ts
- packages/control-plane/test/integration/provider-credentials.test.ts
- packages/web/src/components/web-session-gate.integration.test.tsx
- packages/control-plane/src/db/api-tokens.ts
- .github/workflows/deploy-web.yml
- packages/control-plane/src/auth/oauth-authorization-service.test.ts
- packages/web/src/components/web-session-gate.test.tsx
- packages/control-plane/src/auth/auth-encryption.ts
- packages/web/src/lib/access-control.test.ts
- packages/control-plane/src/auth/providers/oidc-authorization-code-client.ts
- packages/web/package.json
- packages/control-plane/src/auth/oauth-flow-state.ts
- packages/control-plane/src/auth/providers/types.ts
- packages/control-plane/src/auth/subject-verification.ts
- packages/control-plane/src/routes/auth-tokens.ts
- packages/control-plane/test/integration/auth-tokens.test.ts
- packages/control-plane/src/auth/web-session-tokens.ts
- packages/control-plane/src/auth/token-exchange.ts
- packages/control-plane/src/db/browser-auth-sessions.ts
- packages/control-plane/src/db/oauth-authorization-codes.ts
- packages/control-plane/src/db/provider-credentials.ts
Comment on lines
+45
to
+50
| rateLimit: { | ||
| enabled: true, | ||
| window: 60, | ||
| max: 100, | ||
| storage: "memory", | ||
| }, |
There was a problem hiding this comment.
🔒 Security & Privacy | 🟠 Major | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
printf '\n== File context ==\n'
sed -n '1,220p' packages/control-plane/src/auth/user/better-auth.ts
printf '\n== Search for database wiring and rate limit config ==\n'
rg -n 'rateLimit|storage: "memory"|storage: "database"|config\.database|database:' packages/control-plane/src/auth -S
printf '\n== Better Auth references in repo ==\n'
rg -n 'better-auth|Better Auth|rate limit|rateLimit' packages/control-plane -S
printf '\n== Relevant file inventory ==\n'
git ls-files 'packages/control-plane/src/auth/**'Repository: primo-devs/primo-bg-coding-agent
Length of output: 13648
🏁 Script executed:
#!/bin/bash
set -euo pipefail
printf '\n== Browser auth integration tests ==\n'
sed -n '130,230p' packages/control-plane/test/integration/browser-auth.test.ts
printf '\n== Better Auth migration/schema references ==\n'
rg -n 'rate limit|rateLimit|auth_verifications|auth_sessions|auth_accounts|auth_users|migrate|migration' packages/control-plane/test/integration packages/control-plane/src/db packages/control-plane/src/auth -S
printf '\n== D1 schema/migration inventory ==\n'
git ls-files 'packages/control-plane/**/migration*' 'packages/control-plane/**/migrations/**' 'packages/control-plane/**/db/**'Repository: primo-devs/primo-bg-coding-agent
Length of output: 14477
🏁 Script executed:
#!/bin/bash
set -euo pipefail
printf '\n== Search for rate-limit table names and storage options ==\n'
rg -n 'rate[_-]?limit|ratelimit|_rate_limit|rate_limit' packages/control-plane terraform -S
printf '\n== Inspect browser-auth schema test around declared tables ==\n'
sed -n '1,120p' packages/control-plane/test/integration/browser-auth-schema.test.ts
printf '\n== Inspect Better Auth migration test setup ==\n'
sed -n '1,120p' packages/control-plane/test/integration/browser-auth.test.tsRepository: primo-devs/primo-bg-coding-agent
Length of output: 9679
🌐 Web query:
Better Auth rate limit storage database memory migration table Workers D1 docs
💡 Result:
Better Auth supports storing rate limit data in a database (such as Cloudflare D1) as an alternative to the default in-memory storage, which is often unsuitable for serverless environments [1]. To use a database for rate limiting, configure your Better Auth instance as follows: import { betterAuth } from "better-auth"; export const auth = betterAuth({ rateLimit: { storage: "database", // Use "database" instead of default memory modelName: "rateLimit", // Optional, defaults to "rateLimit" }, }); Migration Requirements: 1. When using the built-in Kysely adapter with D1, you can create the necessary rate limit table by running the Better Auth CLI: npx auth@latest migrate [2][1]. 2. If you are using an ORM like Drizzle or Prisma, you must first generate the schema using npx auth@latest generate and then apply it using your ORM's native migration tools [2][1]. 3. For Cloudflare D1 specifically, Better Auth now includes native support, eliminating the need for custom adapter setups [3][4]. Since Better Auth migrations often require database introspection that may be difficult in the Cloudflare runtime, some developers use custom helper scripts to run migrations programmatically if the CLI is not viable in their environment [2][5]. Important Considerations for Cloudflare: - Secondary Storage: If using Cloudflare KV for secondary storage (including rate limiting), note that KV enforces a minimum Time To Live (TTL) of 60 seconds [6][7]. You must configure your rate limit windows to be at least 60 seconds to prevent errors, and you may need to override default settings for specific endpoints that default to shorter windows [8][7]. - Schema: If you manage the schema manually, the rateLimit table requires at least a unique identifier for the key, a counter for requests in the current window, and a timestamp for the last request [1].
Citations:
- 1: https://better-auth.com/docs/concepts/rate-limit
- 2: https://github.com/better-auth/better-auth/blob/9fed16b6/docs/content/docs/concepts/database.mdx
- 3: https://better-auth.com/blog/1-5
- 4: feat: add built-in support for
D1Databasebetter-auth/better-auth#7519 - 5: https://kemalyilmaz.com/blog/setting-up-better-auth-with-cloudflare-workers-d1-kysely/
- 6: https://github.com/zpg6/better-auth-cloudflare
- 7: https://github.com/zpg6/better-auth-cloudflare/blob/master/docs/configuration.md
- 8: https://github.com/zpg6/better-auth-cloudflare/blob/main/README.md
packages/control-plane/src/auth/user/better-auth.ts: use database-backed rate limiting
storage: "memory" only keeps counters per isolate in Workers, so this limit won’t apply consistently across the deployment. Switch to storage: "database" and add the corresponding Better Auth rate-limit migration/table.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@packages/control-plane/src/auth/user/better-auth.ts` around lines 45 - 50,
Update the Better Auth rateLimit configuration to use database-backed storage
instead of memory, and add the corresponding Better Auth rate-limit
migration/table required by the database adapter. Preserve the existing enabled,
window, and max settings.
Comment on lines
+66
to
+76
| socialProviders: { | ||
| ...(config.github | ||
| ? { | ||
| github: { | ||
| ...config.github, | ||
| disableDefaultScope: true, | ||
| }, | ||
| } | ||
| : {}), | ||
| ...(config.google ? { google: config.google } : {}), | ||
| }, |
There was a problem hiding this comment.
🎯 Functional Correctness | 🔴 Critical | ⚡ Quick win
GitHub OAuth needs explicit scopes here.
disableDefaultScope: true removes Better Auth’s default GitHub scopes, and config.github doesn’t add a replacement scope array. That leaves the authorization request without read:user and user:email, so GitHub sign-in won’t be able to resolve the user’s profile/email.
🐛 Proposed fix
github: {
...config.github,
disableDefaultScope: true,
+ scope: ["read:user", "user:email"],
},📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| socialProviders: { | |
| ...(config.github | |
| ? { | |
| github: { | |
| ...config.github, | |
| disableDefaultScope: true, | |
| }, | |
| } | |
| : {}), | |
| ...(config.google ? { google: config.google } : {}), | |
| }, | |
| socialProviders: { | |
| ...(config.github | |
| ? { | |
| github: { | |
| ...config.github, | |
| disableDefaultScope: true, | |
| scope: ["read:user", "user:email"], | |
| }, | |
| } | |
| : {}), | |
| ...(config.google ? { google: config.google } : {}), | |
| }, |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@packages/control-plane/src/auth/user/better-auth.ts` around lines 66 - 76,
Update the GitHub provider configuration in the socialProviders setup to retain
disableDefaultScope: true while explicitly supplying scopes for read:user and
user:email. Preserve any values from config.github and ensure the explicit scope
array is applied so GitHub sign-in can resolve the user profile and email.
Comment on lines
+1
to
+19
| import { readdirSync, readFileSync } from "node:fs"; | ||
| import { DatabaseSync } from "node:sqlite"; | ||
| import { fileURLToPath } from "node:url"; | ||
| import { describe, expect, it } from "vitest"; | ||
|
|
||
| const MIGRATIONS_DIRECTORY = fileURLToPath( | ||
| new URL("../../../../terraform/d1/migrations/", import.meta.url) | ||
| ); | ||
| const BACKFILL_MIGRATION = "0049_backfill_better_auth_accounts.sql"; | ||
|
|
||
| function applyMigrationsBeforeBackfill(db: DatabaseSync): void { | ||
| const migrationFiles = readdirSync(MIGRATIONS_DIRECTORY) | ||
| .filter((file) => /^\d{4}_.+\.sql$/.test(file) && file < BACKFILL_MIGRATION) | ||
| .sort(); | ||
|
|
||
| for (const migrationFile of migrationFiles) { | ||
| db.exec(readFileSync(`${MIGRATIONS_DIRECTORY}/${migrationFile}`, "utf8")); | ||
| } | ||
| } |
There was a problem hiding this comment.
🩺 Stability & Availability | 🟡 Minor | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
fd -H -t f -g '.nvmrc' -g '.tool-versions' .
fd -t f 'package.json' -d 3 --exec jq -r '{path: input_filename, engines, volta}' {} \;
rg -n 'node-version|setup-node' -C2 .github/workflows/ci.ymlRepository: primo-devs/primo-bg-coding-agent
Length of output: 2047
🌐 Web query:
Node.js node:sqlite added in which version DatabaseSync availability release notes
💡 Result:
The DatabaseSync class, along with the node:sqlite module, was added in Node.js version 22.5.0 [1][2]. Upon its initial introduction, the module was experimental and required the --experimental-sqlite command-line flag to be enabled [3][4]. In subsequent releases, the module's experimental status was updated: - Starting in Node.js v23.4.0 and v22.13.0, the node:sqlite module was no longer required to be behind the --experimental-sqlite flag, though it remained classified as experimental [1][5]. - As of later versions, the module has reached a release candidate stability status [1][6].
Citations:
- 1: https://nodejs.org/api/sqlite.html
- 2: https://nodejs.org/download/release/v22.5.1/docs/api/sqlite.html
- 3: https://docs.redhat.com/en/documentation/red_hat_build_of_node.js/22/pdf/release_notes_for_node.js_22/Red_Hat_build_of_Node.js-22-Release_Notes_for_Node.js_22-en-US.pdf
- 4: lib,src,test,doc: add node:sqlite module nodejs/node#53752
- 5: https://nodejs.org/dist/latest-v24.x/docs/api/sqlite.html
- 6: https://nodejs.org/dist/latest/docs/api/sqlite.html
Raise the Node floor for this test
node:sqlite is only available from Node 22.5.0, but the repo still allows Node 22.0.0. Tighten the supported version to >=22.5.0 so this import doesn’t fail in a compliant environment.
🧰 Tools
🪛 OpenGrep (1.25.0)
[ERROR] 17-17: Dynamic command passed to child_process.exec/execSync. Use child_process.execFile or spawn with an argument array instead.
(coderabbit.command-injection.exec-js)
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@packages/control-plane/src/db/browser-auth-legacy-migration.test.ts` around
lines 1 - 19, Update the repository’s Node.js version constraint to require
>=22.5.0, locating the package/toolchain configuration that defines the
supported Node 22 range. Keep the existing version policy unchanged apart from
raising the minimum needed by the node:sqlite import in this test.
Comment on lines
+66
to
+79
| const browserGitHubAccountInfoSchema = z.object({ | ||
| user: z.object({ | ||
| id: z.string().min(1), | ||
| }), | ||
| data: z.object({ | ||
| provider: z.literal("github"), | ||
| issuer: z.literal("https://github.com"), | ||
| subject: z.string().min(1), | ||
| login: githubLoginSchema, | ||
| displayName: z.string().min(1).optional(), | ||
| verifiedEmails: z.array(z.string()), | ||
| primaryEmail: z.string().nullable(), | ||
| }), | ||
| }); |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
🧩 Analysis chain
🌐 Web query:
better-auth 1.6 accountInfo API response shape user.id data.subject issuer
💡 Result:
In Better Auth v1.6, the accountInfo API endpoint is designed to return information about a user's linked provider account [1][2][3]. The response structure is an object containing two primary properties: user and data [1]. The response shape is defined as follows: { "user": { "id": "string", "name": "string", "email": "string", "image": "string", "emailVerified": "boolean" }, "data": { // This object contains the raw profile information returned by the provider [1][4] } } Key observations regarding your query: 1. User ID: The user.id field in the response refers to the internal Better Auth unique identifier for the user record, not the provider-specific identifier [1]. 2. Subject and Issuer: Better Auth v1.6 typically represents account identity through provider-specific identifiers (such as sub or id from an OIDC provider) [5][6]. While OIDC standardizes the sub (subject) claim, the Better Auth accountInfo API does not explicitly wrap these into a top-level user.subject or user.issuer field in the standard schema [1]. Instead, the provider-specific profile data (which includes fields like sub) is returned within the data object [1][4]. 3. Evolution: In later versions (and as part of the architecture shift toward v1.7), the system has moved toward more explicit account scoping using issuer and provider-specific account identifiers [7][8], but the core structure of accountInfo in v1.6 remains focused on the user object and the provider-supplied raw data map [1]. For server-side usage in v1.6, you can call the endpoint using the auth.api object, providing an optional userId in the query to identify the target user if you are operating outside of a standard session context [2][3][4]. top_results: [1][2][3][4][7]
Citations:
- 1: https://github.com/better-auth/better-auth/blob/9fed16b6/packages/better-auth/src/api/routes/account.ts
- 2: feat(account): support server-side
accountInfocalls without session headers better-auth/better-auth#9813 - 3: https://github.com/better-auth/better-auth/releases/tag/v1.6.13
- 4: https://github.com/better-auth/better-auth/blob/fd6b8c13/docs/content/docs/concepts/oauth.mdx
- 5: https://github.com/better-auth/better-auth/releases/tag/v1.6.17
- 6: https://github.com/better-auth/better-auth/blob/main/packages/better-auth/CHANGELOG.md
- 7: feat(auth)!: scope accounts by issuer better-auth/better-auth#10403
- 8: https://better-auth.com/docs/guides/1-7-upgrade-guide
accountInfo.user.id is not the provider subject. Comparing it to account.subject will break browser enrichment; match on data.subject instead, and keep user.id only for the Better Auth user record.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@packages/control-plane/src/session/identity.ts` around lines 66 - 79, Update
browser GitHub account matching to compare account.subject with
browserGitHubAccountInfoSchema’s data.subject, not user.id; retain user.id
exclusively as the Better Auth user record identifier.
Comment on lines
+14
to
+21
| execFileSync("npm", ["run", "build", "-w", "@open-inspect/shared"], { | ||
| cwd: repositoryDirectory, | ||
| stdio: "pipe", | ||
| }); | ||
| execFileSync("npm", ["run", "build"], { | ||
| cwd: packageDirectory, | ||
| stdio: "pipe", | ||
| }); |
There was a problem hiding this comment.
🩺 Stability & Availability | 🟠 Major | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
git ls-files packages/control-plane/src/worker-build.test.ts packages/control-plane/src | sed -n '1,120p'
echo '---'
wc -l packages/control-plane/src/worker-build.test.ts
echo '---'
cat -n packages/control-plane/src/worker-build.test.ts
echo '---'
rg -n "WORKER_BUILD_TIMEOUT_MS|timeout:" packages/control-plane/src packages/shared/src packages/control-plane -g '!**/dist/**' -g '!**/build/**'
echo '---'
node - <<'JS'
const { execFileSync } = require('node:child_process');
console.log(String(execFileSync).includes('timeout'));
JSRepository: primo-devs/primo-bg-coding-agent
Length of output: 13444
Bound each synchronous build subprocess. execFileSync ignores Vitest’s test timeout while it blocks the event loop, so a hung npm run build can stall the job indefinitely. Pass timeout: WORKER_BUILD_TIMEOUT_MS to both calls.
🧰 Tools
🪛 ast-grep (0.44.1)
[warning] Importing child_process exposes a command-execution surface; ensure any command/argument built from input is validated, and prefer execFile/spawn with an argument array over exec.
Context: import { execFileSync } from "node:child_process";
Note: [CWE-78] Improper Neutralization of Special Elements used in an OS Command ('OS Command Injection').
(detect-child-process-typescript)
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@packages/control-plane/src/worker-build.test.ts` around lines 14 - 21, Add
timeout: WORKER_BUILD_TIMEOUT_MS to both execFileSync calls in the worker build
test, covering the shared-package and worker-package npm builds while preserving
their existing cwd and stdio options.
Comment on lines
+55
to
+60
| function trustedClientIp(request: Request): string | null { | ||
| if (process.env.VERCEL === "1") { | ||
| return request.headers.get("X-Vercel-Forwarded-For"); | ||
| } | ||
| return "cf" in request ? request.headers.get("CF-Connecting-IP") : null; | ||
| } |
There was a problem hiding this comment.
🩺 Stability & Availability | 🟡 Minor | ⚡ Quick win
🧩 Analysis chain
🌐 Web query:
Does the Next.js Request object passed to a Route Handler retain the Cloudflare cfproperty when deployed via@opennextjs/cloudflare?
💡 Result:
When using @opennextjs/cloudflare, the standard Next.js Request object passed to a Route Handler does not automatically retain the Cloudflare-specific cf property directly on the request object [1]. Instead, to access Cloudflare-specific metadata and context, you must use the getCloudflareContext utility provided by the @opennextjs/cloudflare package [1]. This function returns an object containing the cf property (which holds the request's Cloudflare metadata), the environment bindings (env), and the execution context (ctx) [2][1]. To use it in a Route Handler, import the utility and call it within your function [1]: import { getCloudflareContext } from "@opennextjs/cloudflare"; export async function GET(request: Request) { const { env, cf, ctx } = getCloudflareContext; // You can now access Cloudflare metadata via the cf object console.log(cf?.country); //... } This approach is necessary because @opennextjs/cloudflare runs Next.js applications in a manner that transforms the request handling, requiring developers to explicitly retrieve the Cloudflare context through the provided adapter utility rather than relying on properties directly attached to the standard Web Request object [1][3].
Citations:
- 1: https://opennext.js.org/cloudflare/bindings
- 2: https://github.com/opennextjs/opennextjs-cloudflare/blob/main/packages/cloudflare/src/api/cloudflare-context.ts
- 3: https://opennext.js.org/cloudflare
Read Cloudflare metadata from getCloudflareContext()
The OpenNext route-handler Request does not expose Cloudflare's cf metadata, so this branch falls back to null in production and CF-Connecting-IP is never trusted on Cloudflare deployments.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@packages/web/src/lib/browser-auth-proxy.ts` around lines 55 - 60, Update
trustedClientIp to obtain Cloudflare metadata through getCloudflareContext()
instead of checking for a cf property on the Request. Preserve the Vercel
X-Vercel-Forwarded-For behavior, and read CF-Connecting-IP from the Cloudflare
context for non-Vercel deployments.
Comment on lines
+90
to
+106
| const timeoutSignal = AbortSignal.timeout(CONTROL_PLANE_FETCH_TIMEOUT_MS); | ||
| const signal = fetchOptions.signal | ||
| ? AbortSignal.any([fetchOptions.signal, timeoutSignal]) | ||
| : timeoutSignal; | ||
| const boundedFetchOptions = { | ||
| ...fetchOptions, | ||
| signal, | ||
| }; | ||
|
|
||
| // On Cloudflare Workers, use the service binding to call the control plane | ||
| const binding = await getServiceBinding(correlationFields); | ||
| if (binding) { | ||
| return binding.fetch(url, fetchOptions); | ||
| return binding.fetch(url, boundedFetchOptions); | ||
| } | ||
|
|
||
| // Fallback: direct fetch (works on Vercel / local dev) | ||
| return fetch(url, fetchOptions); | ||
| } | ||
|
|
||
| /** | ||
| * Make a control-plane request signed with web's own sig1 service | ||
| * credential — never a user token. | ||
| * | ||
| * Reserved for the token endpoints (exchange/refresh): issuance must be | ||
| * reachable only through web's per-service identity. Throws when | ||
| * SERVICE_AUTH_SECRET is not configured; callers treat that as an exchange | ||
| * failure. | ||
| */ | ||
| /** | ||
| * Token calls sit on the sign-in path and the background refresh check — an | ||
| * unresponsive control plane must fail fast into the callers' existing | ||
| * exchange_fallback/request_failed paths, not hang until the platform's own | ||
| * timeout. | ||
| */ | ||
| const SERVICE_FETCH_TIMEOUT_MS = 10_000; | ||
|
|
||
| export async function controlPlaneTokenFetch( | ||
| path: string, | ||
| init: { method: string; body?: string } | ||
| ): Promise<Response> { | ||
| if ( | ||
| init.method !== "POST" || | ||
| (path !== "/auth/tokens/exchange" && path !== "/auth/tokens/refresh") | ||
| ) { | ||
| throw new Error("Service authentication is restricted to token endpoints"); | ||
| } | ||
| const secret = process.env.SERVICE_AUTH_SECRET; | ||
| if (!secret) { | ||
| throw new Error("SERVICE_AUTH_SECRET not configured"); | ||
| } | ||
|
|
||
| const normalizedPath = path.startsWith("/") ? path : `/${path}`; | ||
| const correlation = await getRequestCorrelation(); | ||
| const correlationFields = getCorrelationLogFields(correlation); | ||
| // The signature covers method, path, query, and body hash — not the host — | ||
| // so signing the URL-based form stays valid across the service binding. | ||
| const url = `${getControlPlaneUrl()}${normalizedPath}`; | ||
|
|
||
| const headers = { | ||
| "Content-Type": "application/json", | ||
| ...(await buildServiceAuthHeaders({ | ||
| service: "web", | ||
| secret, | ||
| method: init.method, | ||
| url, | ||
| body: init.body, | ||
| traceId: correlation.traceId, | ||
| })), | ||
| }; | ||
|
|
||
| return dispatchControlPlaneFetch( | ||
| url, | ||
| { | ||
| method: init.method, | ||
| headers, | ||
| body: init.body, | ||
| signal: AbortSignal.timeout(SERVICE_FETCH_TIMEOUT_MS), | ||
| }, | ||
| correlationFields | ||
| ); | ||
| return fetch(url, boundedFetchOptions); |
There was a problem hiding this comment.
🩺 Stability & Availability | 🟠 Major | ⚡ Quick win
Timeout now also bounds streamed response bodies.
AbortSignal.timeout stays armed after headers arrive, so the attachment/media/diff proxies (which return upstream.body as a stream) will have large downloads aborted mid-stream at CONTROL_PLANE_FETCH_TIMEOUT_MS. If streamed passthrough is expected for multi-MB artifacts, consider letting streaming callers opt out (e.g. an option to skip the internal timeout) rather than applying it unconditionally.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@packages/web/src/lib/control-plane-transport.ts` around lines 90 - 106, The
unconditional timeout in the transport path also aborts streamed response bodies
after headers are received. Update the fetch-options flow around
boundedFetchOptions and the control-plane callers that proxy
attachment/media/diff streams so streaming requests can explicitly skip the
internal CONTROL_PLANE_FETCH_TIMEOUT_MS signal, while retaining the timeout by
default for normal requests and preserving caller-provided signal handling.
Comment on lines
+28
to
47
| export async function getServerAuthSession(): Promise<ServerAuthSession | null> { | ||
| const cookieStore = await cookies(); | ||
| const cookieHeader = serializeBrowserSessionCookies(cookieStore.getAll()); | ||
| if (!cookieHeader) return null; | ||
| const response = await dispatchBrowserAuthRequest({ | ||
| method: "GET", | ||
| pathname: "/api/auth/get-session", | ||
| headers: { Cookie: cookieHeader }, | ||
| }); | ||
|
|
||
| if (response.status === 401) return null; | ||
| if (!response.ok) { | ||
| throw new Error(`Browser authentication failed with status ${response.status}`); | ||
| } | ||
|
|
||
| const payload: unknown = await response.json(); | ||
| if (payload === null) return null; | ||
| const session = browserAuthSessionResponseSchema.parse(payload); | ||
| return { user: session.user }; | ||
| } |
There was a problem hiding this comment.
🩺 Stability & Availability | 🟠 Major | ⚡ Quick win
getServerAuthSession can throw for cases its own callers don't catch.
Cookie parsing errors (serializeBrowserSessionCookies duplicate/invalid-value throws), the explicit non-401 status throw (line 40), and browserAuthSessionResponseSchema.parse failures all propagate out of this function. Every consumer shown in the graph evidence (sessions/route.ts GET/POST, automations/route.ts GET/POST, ws-token/route.ts POST, diff/retry, attachments, media routes) calls await getServerAuthSession() before its try/catch block, so any of these throws becomes an unhandled 500 instead of the intended graceful 401/error response. This is new exposure versus the prior NextAuth-backed implementation, which didn't synchronously throw on cookie shape issues.
Consider catching the internal parse/serialize errors here and mapping them to null (unauthenticated) or a well-defined thrown error type that callers are updated to catch, so route handlers keep returning controlled JSON responses.
🛡️ Sketch of a safer wrapper
export async function getServerAuthSession(): Promise<ServerAuthSession | null> {
const cookieStore = await cookies();
- const cookieHeader = serializeBrowserSessionCookies(cookieStore.getAll());
+ let cookieHeader: string | null;
+ try {
+ cookieHeader = serializeBrowserSessionCookies(cookieStore.getAll());
+ } catch {
+ // Malformed/duplicate browser session cookies are treated as unauthenticated.
+ return null;
+ }
if (!cookieHeader) return null;
const response = await dispatchBrowserAuthRequest({
method: "GET",
pathname: "/api/auth/get-session",
headers: { Cookie: cookieHeader },
});
if (response.status === 401) return null;
if (!response.ok) {
throw new Error(`Browser authentication failed with status ${response.status}`);
}
const payload: unknown = await response.json();
if (payload === null) return null;
- const session = browserAuthSessionResponseSchema.parse(payload);
- return { user: session.user };
+ const parsed = browserAuthSessionResponseSchema.safeParse(payload);
+ if (!parsed.success) {
+ throw new Error("Browser session response failed validation");
+ }
+ return { user: parsed.data.user };
}🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@packages/web/src/lib/server-auth-session.ts` around lines 28 - 47, Update
getServerAuthSession to contain failures from serializeBrowserSessionCookies,
the non-success dispatchBrowserAuthRequest response, and
browserAuthSessionResponseSchema.parse so callers receive the intended
unauthenticated result or a consistently handled authentication error instead of
an unhandled exception. Prefer returning null for malformed cookies and invalid
session payloads, and ensure any deliberately thrown error is handled
consistently by all consumers.
Comment on lines
+21
to
+46
| INSERT INTO auth_users ( | ||
| id, | ||
| name, | ||
| email, | ||
| emailVerified, | ||
| image, | ||
| createdAt, | ||
| updatedAt | ||
| ) | ||
| SELECT | ||
| users.id, | ||
| coalesce(nullif(trim(users.display_name), ''), lower(trim(users.email))), | ||
| lower(trim(users.email)), | ||
| 0, | ||
| users.avatar_url, | ||
| strftime('%Y-%m-%dT%H:%M:%fZ', users.created_at / 1000.0, 'unixepoch'), | ||
| strftime('%Y-%m-%dT%H:%M:%fZ', users.updated_at / 1000.0, 'unixepoch') | ||
| FROM users | ||
| WHERE users.email IS NOT NULL | ||
| AND length(trim(users.email)) > 0 | ||
| AND NOT EXISTS ( | ||
| SELECT 1 | ||
| FROM auth_users | ||
| WHERE auth_users.id = users.id | ||
| AND lower(trim(auth_users.email)) = lower(trim(users.email)) | ||
| ); |
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Locate the migration and any schema definitions for auth_users / users.
git ls-files | rg 'terraform/d1/migrations/0049_backfill_better_auth_accounts.sql|auth_users|users|better_auth|schema|migrations'
# Show the relevant migration section with line numbers.
sed -n '1,140p' terraform/d1/migrations/0049_backfill_better_auth_accounts.sql
# Search for auth_users table definition and related constraints.
rg -n -A6 -B6 'CREATE TABLE\s+auth_users|auth_users\s*\(' terraform . -g '*.sql' -g '*.ts' -g '*.js' -g '*.json' -g '*.md'
# Search for any code that updates or syncs users.email into auth_users.email.
rg -n 'auth_users.*email|users.*email|lower\(trim\(users\.email\)\)|lower\(trim\(auth_users\.email\)\)' terraform . -g '*.sql' -g '*.ts' -g '*.js'Repository: primo-devs/primo-bg-coding-agent
Length of output: 23649
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Inspect the user-store write paths that might change users.email.
sed -n '120,220p' packages/control-plane/src/db/user-store.ts
sed -n '320,380p' packages/control-plane/src/db/user-store.ts
# Inspect browser-auth callback / projection code for any auth_users sync on user updates.
sed -n '220,340p' packages/control-plane/test/integration/browser-auth-callback.test.ts
sed -n '1,220p' packages/control-plane/src/db/browser-auth-legacy-migration.test.ts
# Search for UPDATEs to users and auth_users across the repo.
rg -n 'UPDATE\s+users\b|UPDATE\s+auth_users\b|INSERT INTO auth_users|DELETE FROM auth_users|set\s+email\s*=' packages terraform -g '*.ts' -g '*.js' -g '*.sql'
# Check whether any code explicitly keeps auth_users.email in sync with users.email.
rg -n 'auth_users.*users\.email|users\.email.*auth_users|sync.*auth_users|projection' packages terraform -g '*.ts' -g '*.js' -g '*.sql'Repository: primo-devs/primo-bg-coding-agent
Length of output: 19474
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Inspect the full user update path for email mutations.
sed -n '220,300p' packages/control-plane/src/db/user-store.ts
# Inspect the canonical user projection helpers for any auth_users sync behavior.
sed -n '1,220p' packages/control-plane/src/auth/user/canonical-user-projection.ts
sed -n '1,240p' packages/control-plane/src/auth/user/better-auth.ts
# Look for any API or service that updates a user's email independently of auth_users.
rg -n 'updateUser\(|set.*email|email.*update|users\.email|auth_users\.email' packages/control-plane/src packages/control-plane/test -g '*.ts'Repository: primo-devs/primo-bg-coding-agent
Length of output: 10628
Guard the backfill against existing same-id auth_users rows
NOT EXISTS only skips rows when both id and email match. If users.email has changed since auth_users was created for the same id, this insert will hit the primary key and abort the migration. Add ON CONFLICT(id) DO NOTHING here.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@terraform/d1/migrations/0049_backfill_better_auth_accounts.sql` around lines
21 - 46, Add an ON CONFLICT(id) DO NOTHING clause to the INSERT into auth_users
in the backfill migration, preserving the existing SELECT and NOT EXISTS
filtering while preventing duplicate primary-key rows from aborting the
migration.
Comment on lines
+88
to
+103
| hashes = [ | ||
| "h1:OO+IuvQJSPmWdN8AyyIEvPJbLvDQpgX/zbktoa9KsJE=", | ||
| "zh:161ad0bd9a75768c82f53fb6e7172a9d8be2d4889b012645a34795031aaf1bf1", | ||
| "zh:19dc9a5b17729725ccfc4f45b0500af0ee5bc6b6b160c7adb8f2bf617d2c80ea", | ||
| "zh:269eda8fe42daa7974d5a34d166c3ba9defe80cde86c01e4dadcfdf2e1f05e5f", | ||
| "zh:373f7c65566f8f2cc7f45d698654feb9d988996957e1266a69ca00c52d6d16d0", | ||
| "zh:5599d16804c41c83009ec621b6d6b6f74e102f5827678a4750f8809055546b61", | ||
| "zh:583be0440469a22bff70dcfa56593b01566860b29607437264adb51060cf46fc", | ||
| "zh:5f211d8ec3f2e1f414870d9584bfe26e6995560ef81c748f8447a48164767398", | ||
| "zh:78d5eefdd9e494defcb3c68d282b8f96630502cac21d1ea161f53cfe9bb483b3", | ||
| "zh:7b547fd16216761ef86efc3ed516ac5ac0c5c42b7c7eb24a08cef2d93f69ed5e", | ||
| "zh:7e7c0679daf2a382151d05068c8c3f0dae6b7b7dccf818827b73dd08638df2ef", | ||
| "zh:8089dec888a8038b9b4fb23b3df7e1057293dbc5b60b42cc47ff690d69d4b61b", | ||
| "zh:c51f15a031edfd6f23ce8ced3446ca7f8d8d647e2499890d7d5d10d5016d7257", | ||
| "zh:c94784f005708890dc6895afd53636ec00ec1e430b15d41e5aebfb1d4b39bd04", | ||
| ] |
There was a problem hiding this comment.
🩺 Stability & Availability | 🟡 Minor | ⚡ Quick win
Lock entry has only one h1: hash while neighboring providers have two.
hashicorp/random was likely locked on a single platform. terraform init on a different OS/arch will fail checksum verification. Regenerate with terraform providers lock -platform=linux_amd64 -platform=darwin_arm64 … to match the coverage of the null and vercel entries.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@terraform/environments/production/.terraform.lock.hcl` around lines 88 - 103,
Update the hashicorp/random lock entry’s hashes list to include the complete
platform coverage for linux_amd64 and darwin_arm64, regenerating it with
terraform providers lock and preserving the existing checksums alongside the
additional h1 hash.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Automated upstream sync
Clean merge from
ColeMurray/background-agents@main.This PR was opened automatically by
.github/workflows/sync-upstream.yml. Review the commit list and merge when CI is green.Summary by CodeRabbit
New Features
Bug Fixes
Content-Lengthand encoding headers when serving decoded media and attachments.Documentation
Chores